From patchwork Mon Apr 8 15:48:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cole Robinson X-Patchwork-Id: 161939 Delivered-To: patch@linaro.org Received: by 2002:a02:c6d8:0:0:0:0:0 with SMTP id r24csp3843551jan; Mon, 8 Apr 2019 08:48:38 -0700 (PDT) X-Google-Smtp-Source: APXvYqy/HWeDL2nlRpS5gR98GH466FKB/jtBTkEz/x/7xqBG4DP4ByFP/4XABOUSNKa6QD6E0cVO X-Received: by 2002:a0c:af4d:: with SMTP id j13mr24896859qvc.75.1554738518740; Mon, 08 Apr 2019 08:48:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1554738518; cv=none; d=google.com; s=arc-20160816; b=zWmxWuFBSfenzUm/J45VKIFIyl3J2SbLMZhObUQvHHXUICbLg/cz9efFk5WDeNnNfH X4R+Xrgq32Ft6GHQziFzd36Uo2+zjEFD0UvlcyR5rNEuJ5FHh9cggwRlxC+k/ifeoo4F 7SZuQH7KCszaAgbkvIAa5PRNC/UtwU/InW4nuXosdBxhA+Sk7diMUEmQk2V7UwQ74Mrr kelRIITByH5FoY0anu4775s3vb3CLzR3sAt6FKMlrkWYbvqw8KNJbQZvdGEps/E9j0DX BWLokVfoiUe8hxNbAOQrXo70aDmihigESDcU9dGI+Cggaitl58ZEzE8UmYIO3Lw6RGoH F8zw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:sender:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:message-id:date:to:from:delivered-to; bh=yJjhAhn6090bzIVlLFVIm58LOhNaom+IgnLjsynN3jw=; b=YuCe0lZJCF6BVG2cwpAP5rL5bu4/R59Q/Ln6LhGWZkXe6Wv73QvzPr3ATplUVhNIHL advV2ulJ2SX5Vk9/4kgP4Klx2P4mIaoDED3WfV6cWFetxv/+2msVRtPVOsW6sbIMen0+ RynQ9IKllltJy8n8neb3deoEOSvWT8kpBV6TzfbraPYrqXFXqSiG3mn6ocynKXXqQI2z veXyuLaZrcAXZdMq3l++ZNXE83e1P/+5dF7v8jCEMowJkL82mUOR/FzIHTwiSx+btLWc 1o3VfSAWCKDyrqRJ2S5gsBldqKopHSVZ9AXeefn3KOwk7fQiERjpd/Ayg8OfdfCD9zk/ o7Gg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com. [209.132.183.28]) by mx.google.com with ESMTPS id 17si1491795qvg.9.2019.04.08.08.48.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Apr 2019 08:48:38 -0700 (PDT) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 05B28307DAC8; Mon, 8 Apr 2019 15:48:37 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4B325600C7; Mon, 8 Apr 2019 15:48:36 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id A7C1D181AC44; Mon, 8 Apr 2019 15:48:33 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x38FmVNv024551 for ; Mon, 8 Apr 2019 11:48:31 -0400 Received: by smtp.corp.redhat.com (Postfix) id 9F0B25DA35; Mon, 8 Apr 2019 15:48:31 +0000 (UTC) Delivered-To: libvirt-list@redhat.com Received: from worklaptop.redhat.com (ovpn-126-26.rdu2.redhat.com [10.10.126.26]) by smtp.corp.redhat.com (Postfix) with ESMTP id 98A355D960; Mon, 8 Apr 2019 15:48:27 +0000 (UTC) From: Cole Robinson To: libvirt-list@redhat.com Date: Mon, 8 Apr 2019 11:48:15 -0400 Message-Id: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 0/4] Add 'label' arg to VIR_ENUM_IMPL X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Mon, 08 Apr 2019 15:48:37 +0000 (UTC) This is a different take on an RFE I posted in July: https://www.redhat.com/archives/libvir-list/2018-July/msg01815.html The goal of this series is to get us close to being able to raise errors from ToString and FromString functions directly. This will allow us to drop many hundreds of lines of error reporting and accompanying translator load. The first attempt above added a new macro, VIR_ENUM_IMPL_LABEL, which would take a string label, like 'domain type' for virDomainType enum. The generated ToString and FromString functions for that enum would now raise errors containing the label. Redundant error reporting could now be stripped out piece by piece as individual enums are converted. Thinking some more though I don't really like this strategy. Calling code won't have an easy way to know whether ToString/FromString functions are the type that raise errors or not, which could lead to new code accidentally omitting them and failing to raise an error. It could also stretch out the time that the code base is in a half transitioned state. This series is a slightly different direction. VIR_ENUM_IMPL now always takes a label, but for now we don't actually raise any errors. Later at a chosen time we can enable error reporting in the code and begin removing duplicate error reporting. The only downside is that until the transition is complete, there may be cases of double error reporting, but that's the lesser of two evils compared to the previous approach IMO. So the feedback I'm looking for: - Is this worth doing? (danpb+mprivozn said as much for first posting) - Is this approach acceptable? - Review of the strings added in the last patch Notes: - This is on top of pkrempa's virenum.c patches on list - The last two patches are separated to make review easier, but they will be committed together - When enabled, one notable case this will make error reporting worse is virTristate usage. But I think if we manually implement *TOString/FromString wrappers in that case we can require callers to pass in an identifying string. Something to think about for later Cole Robinson (4): Always put _LAST enums on second line of VIR_ENUM_IMPL cfg.mk: Only force _LAST enum on VIR_ENUM_IMPL second line util: Add 'label' field to VIR_ENUM_IMPL Add string labels to all VIR_ENUM_IMPL calls cfg.mk | 6 +- docs/apibuild.py | 12 ++ src/access/viraccessperm.c | 20 +- src/conf/capabilities.c | 3 +- src/conf/cpu_conf.c | 21 +- src/conf/device_conf.c | 3 +- src/conf/domain_capabilities.c | 3 +- src/conf/domain_conf.c | 333 +++++++++++++++++++----------- src/conf/interface_conf.c | 2 +- src/conf/netdev_vlan_conf.c | 3 +- src/conf/network_conf.c | 11 +- src/conf/node_device_conf.c | 12 +- src/conf/numa_conf.c | 7 +- src/conf/nwfilter_conf.c | 21 +- src/conf/snapshot_conf.c | 6 +- src/conf/storage_adapter_conf.c | 2 +- src/conf/storage_conf.c | 18 +- src/libxl/libxl_domain.c | 3 +- src/locking/lock_daemon.c | 3 +- src/logging/log_daemon.c | 3 +- src/lxc/lxc_domain.c | 7 +- src/lxc/lxc_native.c | 3 +- src/network/leaseshelper.c | 3 +- src/qemu/qemu_agent.c | 4 +- src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_command.c | 27 ++- src/qemu/qemu_domain.c | 12 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_firmware.c | 6 +- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_migration_params.c | 9 +- src/qemu/qemu_monitor.c | 8 +- src/qemu/qemu_monitor_json.c | 10 +- src/remote/remote_daemon.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupbackend.c | 3 +- src/util/vircgroupv1.c | 3 +- src/util/vircgroupv2.c | 3 +- src/util/virconf.c | 3 +- src/util/virenum.c | 28 ++- src/util/virenum.h | 15 +- src/util/virerror.c | 3 +- src/util/virfirewall.c | 3 +- src/util/virfirewalld.c | 6 +- src/util/virgic.c | 3 +- src/util/virhook.c | 20 +- src/util/virkeycode.c | 3 +- src/util/virlog.c | 3 +- src/util/virmdev.c | 3 +- src/util/virnetdev.c | 6 +- src/util/virnetdevmacvlan.c | 3 +- src/util/virnetdevvportprofile.c | 6 +- src/util/virpci.c | 9 +- src/util/virperf.c | 3 +- src/util/virprocess.c | 3 +- src/util/virresctrl.c | 12 +- src/util/virsecret.c | 3 +- src/util/virstorageencryption.c | 4 +- src/util/virstoragefile.c | 17 +- src/util/virsysinfo.c | 3 +- src/util/virtypedparam.c | 3 +- src/util/virutil.c | 1 - src/vmware/vmware_conf.c | 3 +- src/vmx/vmx.c | 3 +- tests/group-qemu-caps.pl | 2 +- tests/virkeycodetest.c | 3 +- tools/virsh-domain-monitor.c | 24 +-- tools/virsh-domain.c | 56 ++--- tools/virsh-host.c | 3 +- tools/virsh-network.c | 10 +- tools/virsh-nodedev.c | 2 +- tools/virsh-pool.c | 4 +- tools/virsh-secret.c | 2 +- tools/virsh-volume.c | 5 +- tools/virt-admin.c | 2 +- tools/virt-host-validate-common.c | 3 +- 77 files changed, 561 insertions(+), 334 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list