- Devel - libvirt List Archives

[libvirt] [PATCH] virsh: correct the cmdDomIfGetLink function return value error
by ajia＠redhat.com 09 Dec '11

09 Dec '11

From: Alex Jia <ajia(a)redhat.com> * tools/virsh.c: Although finding interface with matching mac addr, forgot to set 'ret' boolean value to 'true'. * how to reproduce? % virsh domif-getlink <guest name> <interface mac> % echo $? Signed-off-by: Alex Jia <ajia(a)redhat.com> --- tools/virsh.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 276e1cc..91c3459 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1538,6 +1538,7 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) mac = virXMLPropString(cur, "address"); if (STRCASEEQ(mac, iface)){ + ret = true; goto hit; } } -- 1.7.1

3 4

[libvirt] FYI: SpecVirt results using libvirt + KVM
by Daniel P. Berrange 09 Dec '11

09 Dec '11

Some folks may be aware of the SpecVirt benchmark, where KVM is giving the competition something to worry about :-) http://www.spec.org/virt_sc2010/results/ >From the POV of libvirt uses though, there has been one annoyance with the previously submitted results though, in that they have been invoking the KVM command line directly. This has made it harder for people who are deploying KVM using libvirt to replicate the optimal configurations. I'm pleased to be able to point out that the latest result published for KVM by IBM has in fact been done using libvirt! You can clearly see how the guests are being configured with libvirt and the results demonstrate that using libvirt to manage KVM does not involve sacrificing any of the performance that KVM offers: http://www.spec.org/virt_sc2010/results/res2011q4/ This can only continue to get better as our communities work together to add more performance related features to both libvirt and KVM :-) Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

1 0

[libvirt] [PATCH] bridge: Fix forward delay APIs
by Jiri Denemark 09 Dec '11

09 Dec '11

Due to copy&paste error in c1df2c14b590b3d68b707aa4f3a570f95a6bc548, virNetDevBridge[SG]etSTPDelay APIs were accessing wrong file. --- src/util/virnetdevbridge.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index e246b2c..08c8f5c 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -395,7 +395,7 @@ int virNetDevBridgeSetSTPDelay(const char *brname, if ((fd = virNetDevSetupControl(brname, &ifr)) < 0) goto cleanup; - ret = virNetDevBridgeSet(brname, "stp_state", MS_TO_JIFFIES(delay), + ret = virNetDevBridgeSet(brname, "forward_delay", MS_TO_JIFFIES(delay), fd, &ifr); cleanup: @@ -426,7 +426,7 @@ int virNetDevBridgeGetSTPDelay(const char *brname, if ((fd = virNetDevSetupControl(brname, &ifr)) < 0) goto cleanup; - ret = virNetDevBridgeGet(brname, "stp_state", &i, + ret = virNetDevBridgeGet(brname, "forward_delay", &i, fd, &ifr); *delayms = JIFFIES_TO_MS(i); -- 1.7.8

2 1

[libvirt] [PATCH] cpu: Add cpu flags supported by newest qemu
by Peter Krempa 09 Dec '11

09 Dec '11

Add support for newly supported Intel cpu features. Newly supported flags are: pclmuldq, dtes64, smx, fma, pdcm, movbe, xsave, osxsave and avx. This adds support for Intel's Sandy Bridge platform. --- I did not add a new cpu model, as there's no corresponding model in QEMU yet. https://bugzilla.redhat.com/show_bug.cgi?id=761005 src/cpu/cpu_map.xml | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 35dca20..693caf1 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -100,6 +100,12 @@ <feature name='pni'>  <cpuid function='0x00000001' ecx='0x00000001'/> </feature> + <feature name='pclmuldq'> + <cpuid function='0x00000001' ecx='0x00000002'/> + </feature> + <feature name='dtes64'> + <cpuid function='0x00000001' ecx='0x00000004'/> + </feature> <feature name='monitor'>  <cpuid function='0x00000001' ecx='0x00000008'/> </feature> @@ -109,6 +115,9 @@ <feature name='vmx'>  <cpuid function='0x00000001' ecx='0x00000020'/> </feature> + <feature name='smx'> + <cpuid function='0x00000001' ecx='0x00000040'/> + </feature> <feature name='est'>  <cpuid function='0x00000001' ecx='0x00000080'/> </feature> @@ -121,12 +130,18 @@ <feature name='cid'>  <cpuid function='0x00000001' ecx='0x00000400'/> </feature> + <feature name='fma'> + <cpuid function='0x00000001' ecx='0x00001000'/> + </feature> <feature name='cx16'>  <cpuid function='0x00000001' ecx='0x00002000'/> </feature> <feature name='xtpr'>  <cpuid function='0x00000001' ecx='0x00004000'/> </feature> + <feature name='pdcm'> + <cpuid function='0x00000001' ecx='0x00008000'/> + </feature> <feature name='dca'>  <cpuid function='0x00000001' ecx='0x00040000'/> </feature> @@ -139,12 +154,24 @@ <feature name='x2apic'>  <cpuid function='0x00000001' ecx='0x00200000'/> </feature> + <feature name='movbe'> + <cpuid function='0x00000001' ecx='0x00400000'/> + </feature> <feature name='popcnt'>  <cpuid function='0x00000001' ecx='0x00800000'/> </feature> <feature name='aes'> <cpuid function='0x00000001' ecx='0x02000000'/> </feature> + <feature name='xsave'> + <cpuid function='0x00000001' ecx='0x04000000'/> + </feature> + <feature name='osxsave'> + <cpuid function='0x00000001' ecx='0x08000000'/> + </feature> + <feature name='avx'> + <cpuid function='0x00000001' ecx='0x10000000'/> + </feature> <feature name='hypervisor'>  <cpuid function='0x00000001' ecx='0x80000000'/> </feature> -- 1.7.3.4

2 2

Re: [libvirt] how to use macvtap in kvm guest
by Amit Tewari 09 Dec '11

09 Dec '11

Hi, Can somebody help me in solving the dmesg error: macvtap0:no ipv6 routers present ________________________________ From: xhu [mailto:xhu@redhat.com] Sent: Friday, December 09, 2011 10:53 AM To: Amit Tewari Cc: libvir-list(a)redhat.com Subject: Re: [libvirt] how to use macvtap in kvm guest On 12/09/2011 12:28 PM, Amit Tewari wrote: Hi I am creating macvtap0 interface on my eth1 interface. ip link show macvtap0 133: macvtap0@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UNKNOWN qlen 500 link/ether 3e:63:18:f3:49:63 brd ff:ff:ff:ff:ff:ff guest interface file- <interface type='direct'> <mac address='3e:63:18:f3:49:63'/> <source dev='eth0' mode='bridge'/> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </interface> Now when I do- Virsh define guest works fine But when I do virsh start guest can guest gain ip? Dmesg shows error macvtap0:no ipv6 routers present libvirtd[24907]: segfault at 0 ip 0000003e22280062 sp 00007fd3057f8cb8 error 4 in libc-2.12.so[3e22200000+187000] and even if guest starts its eth interface is not created could you provide your host os, guest os, libvirt, qemu-kvm version? it is fine as i tried, no segment fault message and the guest can get ip. DISCLAIMER: ------------------------------------------------------------------------ ----------------------------------------------- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . ------------------------------------------------------------------------ ----------------------------------------------- -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list DISCLAIMER: ----------------------------------------------------------------------------------------------------------------------- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . -----------------------------------------------------------------------------------------------------------------------

1 0

[libvirt] [RFC] BlockJob: Support sync/async virDomainBlockJobAbort
by Adam Litke 09 Dec '11

09 Dec '11

Qemu has changed the semantics of the "block_job_cancel" API. Originally, the operation was synchronous (ie. upon command completion, the operation was guaranteed to be completely stopped). With the new semantics, a "block_job_cancel" merely requests that the operation be cancelled and an event is triggered once the cancellation request has been honored. To adopt the new semantics while preserving compatibility I propose the following updates to the virDomainBlockJob API: A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED will be recognized by libvirt. Regardless of the flags used with virDomainBlockJobAbort, this event will be raised whenever it is received from qemu. This event indicates that a block job has been successfully cancelled. A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC will be added to the virDomainBlockJobAbort API. When enabled, this function will operate asynchronously (ie, it can return before the job has actually been cancelled). When the API is used in this mode, it is the responsibility of the caller to wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the virDomainGetBlockJobInfo API to check the cancellation status. Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally poll using qemu's "query-block-jobs" API and will not return until the operation has been completed. API users are advised that this operation is unbounded and further interaction with the domain during this period may block. This patch implements the new event type, the API flag, and the polling. The main outstanding issue is whether we should bound the amount of time we will wait for cancellation and return an error. Comments on this proposal? Signed-off-by: Adam Litke <agl(a)us.ibm.com> Cc: Stefan Hajnoczi <stefanha(a)gmail.com> Cc: Eric Blake <eblake(a)redhat.com> --- include/libvirt/libvirt.h.in | 10 ++++++++++ src/libvirt.c | 9 ++++++++- src/qemu/qemu_driver.c | 24 +++++++++++++++++------- src/qemu/qemu_monitor.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 36 +++++++++++++++++++++++++++++------- 6 files changed, 90 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2480add..08fc1de 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1677,6 +1677,15 @@ typedef enum { VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1, } virDomainBlockJobType; +/** + * virDomainBlockJobAbortFlags: + * + * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for completion + */ +typedef enum { + VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1, +} virDomainBlockJobAbortFlags; + /* An iterator for monitoring block job operations */ typedef unsigned long long virDomainBlockJobCursor; @@ -3188,6 +3197,7 @@ typedef void (*virConnectDomainEventGraphicsCallback)(virConnectPtr conn, typedef enum { VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0, VIR_DOMAIN_BLOCK_JOB_FAILED = 1, + VIR_DOMAIN_BLOCK_JOB_CANCELLED = 2, } virConnectDomainEventBlockJobStatus; /** diff --git a/src/libvirt.c b/src/libvirt.c index 68074e7..103e246 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17016,7 +17016,7 @@ error: * virDomainBlockJobAbort: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand - * @flags: currently unused, for future extension + * @flags: bitwise or of virDomainBlockJobAbortFlags * * Cancel the active block job on the given disk. * @@ -17027,6 +17027,13 @@ error: * can be found by calling virDomainGetXMLDesc() and inspecting * elements within //domain/devices/disk. * + * By default, this function performs a synchronous operation and the caller + * may assume that the operation has completed when 0 is returned. However, + * BlockJob operations may take a long time to complete, and during this time + * further domain interactions may be unresponsive. To avoid this problem, pass + * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC in the flags argument to enable asynchronous + * behavior. When the job has been cancelled, a BlockJob event will be emitted. + * * Returns -1 in case of failure, 0 when successful. */ int virDomainBlockJobAbort(virDomainPtr dom, const char *disk, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e5ed9a..18c41d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10947,7 +10947,7 @@ cleanup: static int qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, unsigned long bandwidth, virDomainBlockJobInfoPtr info, - int mode) + int flags, int mode) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; @@ -10988,6 +10988,15 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, qemuDomainObjEnterMonitorWithDriver(driver, vm); priv = vm->privateData; ret = qemuMonitorBlockJob(priv->mon, device, bandwidth, info, mode); + /* + * Qemu provides asynchronous block job cancellation but without the + * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag libvirt guarantees a synchronous + * operation. Provide this behavior by waiting here (with the monitor + * locked) so we don't get confused by newly scheduled block jobs. + */ + if (ret == 0 && mode == BLOCK_JOB_ABORT && + !(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) + ret = qemuMonitorBlockJobCancelWait(priv->mon, device); qemuDomainObjExitMonitorWithDriver(driver, vm); endjob: @@ -11007,8 +11016,7 @@ cleanup: static int qemuDomainBlockJobAbort(virDomainPtr dom, const char *path, unsigned int flags) { - virCheckFlags(0, -1); - return qemuDomainBlockJobImpl(dom, path, 0, NULL, BLOCK_JOB_ABORT); + return qemuDomainBlockJobImpl(dom, path, 0, NULL, flags, BLOCK_JOB_ABORT); } static int @@ -11016,7 +11024,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const char *path, virDomainBlockJobInfoPtr info, unsigned int flags) { virCheckFlags(0, -1); - return qemuDomainBlockJobImpl(dom, path, 0, info, BLOCK_JOB_INFO); + return qemuDomainBlockJobImpl(dom, path, 0, info, flags, BLOCK_JOB_INFO); } static int @@ -11024,7 +11032,8 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags) { virCheckFlags(0, -1); - return qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_SPEED); + return qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, flags, + BLOCK_JOB_SPEED); } static int @@ -11034,9 +11043,10 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, int ret; virCheckFlags(0, -1); - ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_PULL); + ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, flags, + BLOCK_JOB_PULL); if (ret == 0 && bandwidth != 0) - ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, + ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, flags, BLOCK_JOB_SPEED); return ret; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4141fb7..040ada8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2564,6 +2564,30 @@ int qemuMonitorScreendump(qemuMonitorPtr mon, return ret; } +/* Poll the monitor to wait for the block job on a given disk to end. + * We don't need to worry about another block job starting since we have the + * driver locked. */ +int +qemuMonitorBlockJobCancelWait(qemuMonitorPtr mon, const char *device) +{ + VIR_DEBUG("mon=%p, device=%p", mon, device); + /* XXX: Should we provide some sort of escape hatch for this wait? */ + while (1) { + /* Poll every 100ms */ + int ret; + struct timespec ts = { .tv_sec = 0, .tv_nsec = 100 * 1000 * 1000ull }; + virDomainBlockJobInfo info; + + ret = qemuMonitorBlockJob(mon, device, 0, &info, BLOCK_JOB_INFO); + if (ret < 0) + return -1; + else if (ret == 0) + return 0; + else + nanosleep(&ts, NULL); + } +} + int qemuMonitorBlockJob(qemuMonitorPtr mon, const char *device, unsigned long bandwidth, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 15acf8b..afc081e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -510,6 +510,8 @@ typedef enum { BLOCK_JOB_PULL = 3, } BLOCK_JOB_CMD; +int qemuMonitorBlockJobCancelWait(qemuMonitorPtr mon, const char *device); + int qemuMonitorBlockJob(qemuMonitorPtr mon, const char *device, unsigned long bandwidth, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1ef3e84..8618c9a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -57,7 +57,8 @@ static void qemuMonitorJSONHandleIOError(qemuMonitorPtr mon, virJSONValuePtr dat static void qemuMonitorJSONHandleVNCConnect(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValuePtr data); -static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleBlockJobCancelled(qemuMonitorPtr mon, virJSONValuePtr data); struct { const char *type; @@ -73,7 +74,8 @@ struct { { "VNC_CONNECTED", qemuMonitorJSONHandleVNCConnect, }, { "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, }, { "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, }, - { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJob, }, + { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, }, + { "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCancelled, }, }; @@ -685,13 +687,14 @@ static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValueP qemuMonitorJSONHandleVNC(mon, data, VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT); } -static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr data) +static void qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon, + virJSONValuePtr data, + int event) { const char *device; const char *type_str; int type = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; unsigned long long offset, len; - int status = VIR_DOMAIN_BLOCK_JOB_FAILED; if ((device = virJSONValueObjectGetString(data, "device")) == NULL) { VIR_WARN("missing device in block job event"); @@ -716,13 +719,32 @@ static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr da if (STREQ(type_str, "stream")) type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; - if (offset != 0 && offset == len) - status = VIR_DOMAIN_BLOCK_JOB_COMPLETED; + switch (event) { + case VIR_DOMAIN_BLOCK_JOB_COMPLETED: + /* Make sure the whole device has been processed */ + if (offset != len) + event = VIR_DOMAIN_BLOCK_JOB_FAILED; + break; + case VIR_DOMAIN_BLOCK_JOB_FAILED: + case VIR_DOMAIN_BLOCK_JOB_CANCELLED: + break; + } out: - qemuMonitorEmitBlockJob(mon, device, type, status); + qemuMonitorEmitBlockJob(mon, device, type, event); +} + +static void qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + qemuMonitorJSONHandleBlockJobImpl(mon, data, VIR_DOMAIN_BLOCK_JOB_COMPLETED); } +static void qemuMonitorJSONHandleBlockJobCancelled(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + qemuMonitorJSONHandleBlockJobImpl(mon, data, VIR_DOMAIN_BLOCK_JOB_CANCELLED); +} int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, -- 1.7.5.rc1

2 1

[libvirt] how to use macvtap in kvm guest
by Amit Tewari 09 Dec '11

09 Dec '11

Hi I am creating macvtap0 interface on my eth1 interface. ip link show macvtap0 133: macvtap0@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UNKNOWN qlen 500 link/ether 3e:63:18:f3:49:63 brd ff:ff:ff:ff:ff:ff guest interface file- <interface type='direct'> <mac address='3e:63:18:f3:49:63'/> <source dev='eth0' mode='bridge'/> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </interface> Now when I do- Virsh define guest works fine But when I do virsh start guest Dmesg shows error macvtap0:no ipv6 routers present libvirtd[24907]: segfault at 0 ip 0000003e22280062 sp 00007fd3057f8cb8 error 4 in libc-2.12.so[3e22200000+187000] and even if guest starts its eth interface is not created DISCLAIMER: ----------------------------------------------------------------------------------------------------------------------- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . -----------------------------------------------------------------------------------------------------------------------

2 1

[libvirt] [PATCH] virsh: plug memory leak on cmdBlkdeviotune() sucessful path
by ajia＠redhat.com 09 Dec '11

09 Dec '11

From: Alex Jia <ajia(a)redhat.com> Detected by valgrind. Leak introduced in commit e9bd9a0: * tools/virsh.c: fix memory leak on cmdBlkdeviotune. * how to reproduce? % valgrind -v --leak-check=full virsh blkdeviotune <domain name> <block device> * actual valgrind result: ==12759== 576 bytes in 1 blocks are definitely lost in loss record 18 of 29 ==12759== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==12759== by 0x42134E: _vshCalloc.clone.2 (virsh.c:422) ==12759== by 0x4217CB: cmdBlkdeviotune (virsh.c:6364) ==12759== by 0x4136A2: vshCommandRun (virsh.c:16363) ==12759== by 0x4253FB: main (virsh.c:17865) ==12759== ==12759== LEAK SUMMARY: ==12759== definitely lost: 576 bytes in 1 blocks ==12759== indirectly lost: 0 bytes in 0 blocks ==12759== possibly lost: 0 bytes in 0 blocks ==12759== still reachable: 126,964 bytes in 1,342 blocks ==12759== suppressed: 0 bytes in 0 blocks Signed-off-by: Alex Jia <ajia(a)redhat.com> --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index a51478f..e6e4f8b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6400,8 +6400,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) } } - virDomainFree(dom); - return true; + ret = true; + goto cleanup; } else { /* Set the block I/O throttle, match by opt since parameters can be 0 */ params = vshCalloc(ctl, nparams, sizeof(*params)); -- 1.7.1

4 6

[libvirt] [PATCH] nwfilter: cleanup return codes in nwfilter subsystem
by Stefan Berger 09 Dec '11

09 Dec '11

This patch cleans up return codes in the nwfilter subsystem. Some functions in nwfilter_conf.c (validators and formatters) are keeping their bool return for now and I am converting their return code to true/false. All other functions now return -1 on failure and 0 on success. [I searched for all occurences of ' 1;' and checked all 'if ' and adapted where needed. After that I did a grep for 'NWFilter' in the source tree.] --- src/conf/nwfilter_conf.c | 127 ++++++------- src/conf/nwfilter_params.c | 22 +- src/nwfilter/nwfilter_driver.c | 2 src/nwfilter/nwfilter_ebiptables_driver.c | 286 +++++++++++++++--------------- src/nwfilter/nwfilter_gentech_driver.c | 78 ++++---- src/nwfilter/nwfilter_learnipaddr.c | 47 ++-- src/qemu/qemu_command.c | 3 src/qemu/qemu_process.c | 2 src/uml/uml_conf.c | 2 9 files changed, 290 insertions(+), 279 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -214,23 +214,24 @@ static const char state_str[] = " * @attr: The attribute to look up * @res: Pointer to string pointer for result * - * Returns 1 if value was found with result returned, 0 otherwise. + * Returns 0 if value was found with result returned, -1 otherwise. * * lookup a map entry given the integer. */ -static bool +static int intMapGetByInt(const struct int_map *intmap, int32_t attr, const char **res) { int i = 0; - bool found = 0; + int found = false; + while (intmap[i].val && !found) { if (intmap[i].attr == attr) { *res = intmap[i].val; - found = 1; + found = true; } i++; } - return found; + return (found) ? 0 : -1; } @@ -241,26 +242,27 @@ intMapGetByInt(const struct int_map *int * @casecmp : Whether to ignore case when doing string matching * @result: Pointer to int for result * - * Returns 0 if no entry was found, 1 otherwise. + * Returns 0 if entry was found, -1 otherwise. * * Do a lookup in the map trying to find an integer key using the string - * value. Returns 1 if entry was found with result returned, 0 otherwise. + * value. Returns 0 if entry was found with result returned, -1 otherwise. */ -static bool +static int intMapGetByString(const struct int_map *intmap, const char *str, int casecmp, int32_t *result) { int i = 0; - bool found = 0; + bool found = false; + while (intmap[i].val && !found) { if ( (casecmp && STRCASEEQ(intmap[i].val, str)) || STREQ (intmap[i].val, str) ) { *result = intmap[i].attr; - found = 1; + found = true; } i++; } - return found; + return (found) ? 0 : -1; } @@ -367,14 +369,14 @@ virNWFilterRuleDefAddVar(virNWFilterRule if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1) < 0) { virReportOOMError(); - return 1; + return -1; } nwf->vars[nwf->nvars] = strdup(var); if (!nwf->vars[nwf->nvars]) { virReportOOMError(); - return 1; + return -1; } item->var = nwf->vars[nwf->nvars++]; @@ -479,7 +481,7 @@ checkMacProtocolID(enum attrDatatype dat int32_t res = -1; if (datatype == DATATYPE_STRING) { - if (intMapGetByString(macProtoMap, value->c, 1, &res) == 0) + if (intMapGetByString(macProtoMap, value->c, 1, &res) < 0) res = -1; datatype = DATATYPE_UINT16; } else if (datatype == DATATYPE_UINT16 || @@ -492,10 +494,10 @@ checkMacProtocolID(enum attrDatatype dat if (res != -1) { nwf->p.ethHdrFilter.dataProtocolID.u.u16 = res; nwf->p.ethHdrFilter.dataProtocolID.datatype = datatype; - return 1; + return true; } - return 0; + return false; } @@ -509,7 +511,7 @@ macProtocolIDFormatter(virBufferPtr buf, if (intMapGetByInt(macProtoMap, nwf->p.ethHdrFilter.dataProtocolID.u.u16, - &str)) { + &str) == 0) { virBufferAdd(buf, str, -1); } else { if (nwf->p.ethHdrFilter.dataProtocolID.datatype == DATATYPE_UINT16) @@ -517,7 +519,7 @@ macProtocolIDFormatter(virBufferPtr buf, virBufferAsprintf(buf, asHex ? "0x%x" : "%d", nwf->p.ethHdrFilter.dataProtocolID.u.u16); } - return 1; + return true; } @@ -550,7 +552,7 @@ checkVlanProtocolID(enum attrDatatype da int32_t res = -1; if (datatype == DATATYPE_STRING) { - if (intMapGetByString(macProtoMap, value->c, 1, &res) == 0) + if (intMapGetByString(macProtoMap, value->c, 1, &res) < 0) res = -1; datatype = DATATYPE_UINT16; } else if (datatype == DATATYPE_UINT16 || @@ -579,7 +581,7 @@ vlanProtocolIDFormatter(virBufferPtr buf if (intMapGetByInt(macProtoMap, nwf->p.vlanHdrFilter.dataVlanEncap.u.u16, - &str)) { + &str) == 0) { virBufferAdd(buf, str, -1); } else { if (nwf->p.vlanHdrFilter.dataVlanEncap.datatype == DATATYPE_UINT16) @@ -607,7 +609,7 @@ checkValidMask(unsigned char *data, int checkones = 0; } else { if ((data[idx>>3] & mask)) - return 0; + return false; } idx++; @@ -615,7 +617,7 @@ checkValidMask(unsigned char *data, int if (!mask) mask = 0x80; } - return 1; + return true; } @@ -655,7 +657,7 @@ arpOpcodeValidator(enum attrDatatype dat int32_t res = -1; if (datatype == DATATYPE_STRING) { - if (intMapGetByString(arpOpcodeMap, value->c, 1, &res) == 0) + if (intMapGetByString(arpOpcodeMap, value->c, 1, &res) < 0) res = -1; datatype = DATATYPE_UINT16; } else if (datatype == DATATYPE_UINT16 || @@ -666,9 +668,9 @@ arpOpcodeValidator(enum attrDatatype dat if (res != -1) { nwf->p.arpHdrFilter.dataOpcode.u.u16 = res; nwf->p.arpHdrFilter.dataOpcode.datatype = datatype; - return 1; + return true; } - return 0; + return false; } @@ -681,12 +683,12 @@ arpOpcodeFormatter(virBufferPtr buf, if (intMapGetByInt(arpOpcodeMap, nwf->p.arpHdrFilter.dataOpcode.u.u16, - &str)) { + &str) == 0) { virBufferAdd(buf, str, -1); } else { virBufferAsprintf(buf, "%d", nwf->p.arpHdrFilter.dataOpcode.u.u16); } - return 1; + return true; } @@ -708,15 +710,16 @@ static const struct int_map ipProtoMap[] }; -static bool checkIPProtocolID(enum attrDatatype datatype, - union data *value, - virNWFilterRuleDefPtr nwf, - nwItemDesc *item ATTRIBUTE_UNUSED) +static bool +checkIPProtocolID(enum attrDatatype datatype, + union data *value, + virNWFilterRuleDefPtr nwf, + nwItemDesc *item ATTRIBUTE_UNUSED) { int32_t res = -1; if (datatype == DATATYPE_STRING) { - if (intMapGetByString(ipProtoMap, value->c, 1, &res) == 0) + if (intMapGetByString(ipProtoMap, value->c, 1, &res) < 0) res = -1; datatype = DATATYPE_UINT8_HEX; } else if (datatype == DATATYPE_UINT8 || @@ -727,9 +730,9 @@ static bool checkIPProtocolID(enum attrD if (res != -1) { nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8 = res; nwf->p.ipHdrFilter.ipHdr.dataProtocolID.datatype = datatype; - return 1; + return true; } - return 0; + return false; } @@ -743,7 +746,7 @@ formatIPProtocolID(virBufferPtr buf, if (intMapGetByInt(ipProtoMap, nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8, - &str)) { + &str) == 0) { virBufferAdd(buf, str, -1); } else { if (nwf->p.ipHdrFilter.ipHdr.dataProtocolID.datatype == DATATYPE_UINT8) @@ -751,7 +754,7 @@ formatIPProtocolID(virBufferPtr buf, virBufferAsprintf(buf, asHex ? "0x%x" : "%d", nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8); } - return 1; + return true; } @@ -762,11 +765,11 @@ dscpValidator(enum attrDatatype datatype { uint8_t dscp = val->ui; if (dscp > 63) - return 0; + return false; nwf->p.ipHdrFilter.ipHdr.dataDSCP.datatype = datatype; - return 1; + return true; } @@ -805,7 +808,7 @@ parseStringItems(const struct int_map *i } } if (!found) { - rc = 1; + rc = -1; break; } } @@ -874,15 +877,15 @@ stateValidator(enum attrDatatype datatyp char *input = val->c; int32_t flags = 0; - if (parseStateMatch(input, &flags)) - return 0; + if (parseStateMatch(input, &flags) < 0) + return false; item->u.u16 = flags; nwf->flags |= flags; item->datatype = DATATYPE_UINT16; - return 1; + return true; } @@ -1663,13 +1666,11 @@ static const virAttributes virAttr[] = { }; -static bool +static int virNWMACAddressParser(const char *input, nwMACAddressPtr output) { - if (virParseMacAddr(input, &output->addr[0]) == 0) - return 1; - return 0; + return virParseMacAddr(input, &output->addr[0]); } @@ -1714,7 +1715,7 @@ virNWFilterRuleDetailsParse(xmlNodePtr n flags_set |= NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR; if (virNWFilterRuleDefAddVar(nwf, item, - &prop[1])) + &prop[1]) < 0) rc = -1; found = 1; } @@ -1805,8 +1806,8 @@ virNWFilterRuleDetailsParse(xmlNodePtr n break; case DATATYPE_MACADDR: - if (!virNWMACAddressParser(prop, - &item->u.macaddr)) { + if (virNWMACAddressParser(prop, + &item->u.macaddr) < 0) { rc = -1; } found = 1; @@ -1814,8 +1815,8 @@ virNWFilterRuleDetailsParse(xmlNodePtr n case DATATYPE_MACMASK: validator = checkMACMask; - if (!virNWMACAddressParser(prop, - &item->u.macaddr)) { + if (virNWMACAddressParser(prop, + &item->u.macaddr) < 0) { rc = -1; } data.v = &item->u.macaddr; @@ -2418,8 +2419,8 @@ virNWFilterDefParseXML(xmlXPathContextPt } else { /* assign default priority if none can be found via lookup */ if (!name_prefix || - !intMapGetByString(chain_priorities, name_prefix, 0, - &ret->chainPriority)) { + intMapGetByString(chain_priorities, name_prefix, 0, + &ret->chainPriority) < 0) { /* assign default chain priority */ ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY + NWFILTER_MIN_FILTER_PRIORITY) / 2; @@ -2620,7 +2621,7 @@ int virNWFilterSaveConfig(const char *co if (!(xml = virNWFilterDefFormat(def))) goto cleanup; - if (virNWFilterSaveXML(configDir, def, xml)) + if (virNWFilterSaveXML(configDir, def, xml) < 0) goto cleanup; ret = 0; @@ -2649,7 +2650,7 @@ _virNWFilterDefLoopDetect(virConnectPtr if (entry->include) { if (STREQ(filtername, entry->include->filterref)) { - rc = 1; + rc = -1; break; } @@ -2660,8 +2661,8 @@ _virNWFilterDefLoopDetect(virConnectPtr obj->def, filtername); virNWFilterObjUnlock(obj); - if (rc) - break; + if (rc < 0) + break; } } } @@ -2679,7 +2680,7 @@ _virNWFilterDefLoopDetect(virConnectPtr * Detect a loop introduced through the filters being able to * reference each other. * - * Returns 0 in case no loop was detected, 1 otherwise. + * Returns 0 in case no loop was detected, -1 otherwise. */ static int virNWFilterDefLoopDetect(virConnectPtr conn, @@ -2736,7 +2737,7 @@ virNWFilterTriggerVMFilterRebuild(virCon }; if (!cb.skipInterfaces) - return 1; + return -1; for (i = 0; i < nCallbackDriver; i++) { callbackDrvArray[i]->vmFilterRebuild(conn, @@ -2778,7 +2779,7 @@ virNWFilterTestUnassignDef(virConnectPtr nwfilter->wantRemoved = 1; /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild(conn)) - rc = 1; + rc = -1; nwfilter->wantRemoved = 0; @@ -2807,7 +2808,7 @@ virNWFilterObjAssignDef(virConnectPtr co virNWFilterObjUnlock(nwfilter); } - if (virNWFilterDefLoopDetect(conn, nwfilters, def)) { + if (virNWFilterDefLoopDetect(conn, nwfilters, def) < 0) { virNWFilterReportError(VIR_ERR_OPERATION_FAILED, "%s", _("filter would introduce a loop")); return NULL; @@ -3297,8 +3298,8 @@ int virNWFilterConfLayerInit(virHashIter initialized = true; - if (virMutexInitRecursive(&updateMutex)) - return 1; + if (virMutexInitRecursive(&updateMutex) < 0) + return -1; return 0; } Index: libvirt-acl/src/nwfilter/nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -384,7 +384,7 @@ nwfilterUndefine(virNWFilterPtr obj) { goto cleanup; } - if (virNWFilterTestUnassignDef(obj->conn, nwfilter)) { + if (virNWFilterTestUnassignDef(obj->conn, nwfilter) < 0) { virNWFilterReportError(VIR_ERR_OPERATION_INVALID, "%s", _("nwfilter is in use")); Index: libvirt-acl/src/conf/nwfilter_params.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_params.c +++ libvirt-acl/src/conf/nwfilter_params.c @@ -82,7 +82,7 @@ virNWFilterVarValueCopy(const virNWFilte } break; case NWFILTER_VALUE_TYPE_ARRAY: - if (VIR_ALLOC_N(res->u.array.values, val->u.array.nValues)) + if (VIR_ALLOC_N(res->u.array.values, val->u.array.nValues) < 0) goto err_exit; res->u.array.nValues = val->u.array.nValues; for (i = 0; i < val->u.array.nValues; i++) { @@ -490,7 +490,7 @@ hashDataFree(void *payload, const void * * @val: The value associated with the key * @freeName: Whether the name must be freed on table destruction * - * Returns 0 on success, 1 on failure. + * Returns 0 on success, -1 on failure. * * Put an entry into the hashmap replacing and freeing an existing entry * if one existed. @@ -505,25 +505,25 @@ virNWFilterHashTablePut(virNWFilterHashT if (copyName) { name = strdup(name); if (!name) - return 1; + return -1; if (VIR_REALLOC_N(table->names, table->nNames + 1) < 0) { VIR_FREE(name); - return 1; + return -1; } table->names[table->nNames++] = (char *)name; } - if (virHashAddEntry(table->hashTable, name, val) != 0) { + if (virHashAddEntry(table->hashTable, name, val) < 0) { if (copyName) { VIR_FREE(name); table->nNames--; } - return 1; + return -1; } } else { - if (virHashUpdateEntry(table->hashTable, name, val) != 0) { - return 1; + if (virHashUpdateEntry(table->hashTable, name, val) < 0) { + return -1; } } return 0; @@ -614,7 +614,7 @@ addToTable(void *payload, const void *na return; } - if (virNWFilterHashTablePut(atts->target, (const char *)name, val, 1) != 0) { + if (virNWFilterHashTablePut(atts->target, (const char *)name, val, 1) < 0){ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("Could not put variable '%s' into hashmap"), (const char *)name); @@ -640,7 +640,7 @@ virNWFilterHashTablePutAll(virNWFilterHa return 0; err_exit: - return 1; + return -1; } @@ -700,7 +700,7 @@ virNWFilterParseParamAttributes(xmlNodeP value = virNWFilterParseVarValue(val); if (!value) goto skip_entry; - if (virNWFilterHashTablePut(table, nam, value, 1)) + if (virNWFilterHashTablePut(table, nam, value, 1) < 0) goto err_exit; } value = NULL; Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c @@ -106,7 +106,7 @@ virNWFilterRuleInstAddData(virNWFilterRu { if (VIR_REALLOC_N(res->data, res->ndata+1) < 0) { virReportOOMError(); - return 1; + return -1; } res->data[res->ndata++] = data; return 0; @@ -151,28 +151,28 @@ virNWFilterVarHashmapAddStdValues(virNWF if (macaddr) { val = virNWFilterVarValueCreateSimple(macaddr); if (!val) - return 1; + return -1; if (virHashAddEntry(table->hashTable, NWFILTER_STD_VAR_MAC, val) < 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add variable 'MAC' to hashmap")); - return 1; + return -1; } } if (ipaddr) { val = virNWFilterVarValueCopy(ipaddr); if (!val) - return 1; + return -1; if (virHashAddEntry(table->hashTable, NWFILTER_STD_VAR_IP, val) < 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add variable 'IP' to hashmap")); - return 1; + return -1; } } @@ -200,7 +200,7 @@ virNWFilterCreateVarHashmap(char *macadd return NULL; } - if (virNWFilterVarHashmapAddStdValues(table, macaddr, ipaddr)) { + if (virNWFilterVarHashmapAddStdValues(table, macaddr, ipaddr) < 0) { virNWFilterHashTableFree(table); return NULL; } @@ -339,10 +339,10 @@ virNWFilterCreateVarsFrom(virNWFilterHas return NULL; } - if (virNWFilterHashTablePutAll(vars1, res)) + if (virNWFilterHashTablePutAll(vars1, res) < 0) goto err_exit; - if (virNWFilterHashTablePutAll(vars2, res)) + if (virNWFilterHashTablePutAll(vars2, res) < 0) goto err_exit; return res; @@ -404,13 +404,13 @@ _virNWFilterInstantiateRec(virNWFilterTe ifname, vars); if (!inst) { - rc = 1; + rc = -1; break; } if (VIR_REALLOC_N(*insts, (*nEntries)+1) < 0) { virReportOOMError(); - rc = 1; + rc = -1; break; } @@ -425,7 +425,7 @@ _virNWFilterInstantiateRec(virNWFilterTe virNWFilterReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), inc->filterref); - rc = 1; + rc = -1; virNWFilterObjUnlock(obj); break; } @@ -436,7 +436,7 @@ _virNWFilterInstantiateRec(virNWFilterTe vars); if (!tmpvars) { virReportOOMError(); - rc = 1; + rc = -1; virNWFilterObjUnlock(obj); break; } @@ -467,13 +467,13 @@ _virNWFilterInstantiateRec(virNWFilterTe virNWFilterHashTableFree(tmpvars); virNWFilterObjUnlock(obj); - if (rc) + if (rc < 0) break; } else { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("referenced filter '%s' is missing"), inc->filterref); - rc = 1; + rc = -1; break; } } @@ -504,7 +504,7 @@ virNWFilterDetermineMissingVarsRec(virNW if (!virHashLookup(vars->hashTable, rule->vars[j])) { val = virNWFilterVarValueCreateSimpleCopyValue("1"); if (!val) { - rc = 1; + rc = -1; break; } virNWFilterHashTablePut(missing_vars, rule->vars[j], @@ -522,7 +522,7 @@ virNWFilterDetermineMissingVarsRec(virNW virNWFilterReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), inc->filterref); - rc = 1; + rc = -1; virNWFilterObjUnlock(obj); break; } @@ -533,7 +533,7 @@ virNWFilterDetermineMissingVarsRec(virNW vars); if (!tmpvars) { virReportOOMError(); - rc = 1; + rc = -1; virNWFilterObjUnlock(obj); break; } @@ -559,13 +559,13 @@ virNWFilterDetermineMissingVarsRec(virNW virNWFilterHashTableFree(tmpvars); virNWFilterObjUnlock(obj); - if (rc) + if (rc < 0) break; } else { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("referenced filter '%s' is missing"), inc->filterref); - rc = 1; + rc = -1; break; } } @@ -592,7 +592,7 @@ virNWFilterRuleInstancesToArray(int nEnt if (VIR_ALLOC_N((*ptrs), (*nptrs)) < 0) { virReportOOMError(); - return 1; + return -1; } (*nptrs) = 0; @@ -649,7 +649,7 @@ virNWFilterInstantiate(virNWFilterTechDr virNWFilterHashTablePtr missing_vars = virNWFilterHashTableCreate(0); if (!missing_vars) { virReportOOMError(); - rc = 1; + rc = -1; goto err_exit; } @@ -658,7 +658,7 @@ virNWFilterInstantiate(virNWFilterTechDr missing_vars, useNewFilter, driver); - if (rc) + if (rc < 0) goto err_exit; if (virHashSize(missing_vars->hashTable) == 1) { @@ -693,7 +693,7 @@ virNWFilterInstantiate(virNWFilterTechDr useNewFilter, foundNewFilter, driver); - if (rc) + if (rc < 0) goto err_exit; switch (useNewFilter) { @@ -709,10 +709,10 @@ virNWFilterInstantiate(virNWFilterTechDr rc = virNWFilterRuleInstancesToArray(nEntries, insts, &ptrs, &nptrs); - if (rc) + if (rc < 0) goto err_exit; - if (virNWFilterLockIface(ifname)) + if (virNWFilterLockIface(ifname) < 0) goto err_exit; rc = techdriver->applyNewRules(ifname, nptrs, ptrs); @@ -724,7 +724,7 @@ virNWFilterInstantiate(virNWFilterTechDr virResetLastError(); /* interface changed/disppeared */ techdriver->allTeardown(ifname); - rc = 1; + rc = -1; } virNWFilterUnlockIface(ifname); @@ -752,7 +752,7 @@ err_unresolvable_vars: VIR_FREE(buf); } - rc = 1; + rc = -1; goto err_exit; } @@ -792,7 +792,7 @@ __virNWFilterInstantiateFilter(bool tear _("Could not get access to ACL tech " "driver '%s'"), drvname); - return 1; + return -1; } VIR_DEBUG("filter name: %s", filtername); @@ -802,14 +802,14 @@ __virNWFilterInstantiateFilter(bool tear virNWFilterReportError(VIR_ERR_NO_NWFILTER, _("Could not find filter '%s'"), filtername); - return 1; + return -1; } if (obj->wantRemoved) { virNWFilterReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), filtername); - rc = 1; + rc = -1; goto err_exit; } @@ -817,7 +817,7 @@ __virNWFilterInstantiateFilter(bool tear str_macaddr = strdup(vmmacaddr); if (!str_macaddr) { virReportOOMError(); - rc = 1; + rc = -1; goto err_exit; } @@ -825,7 +825,7 @@ __virNWFilterInstantiateFilter(bool tear vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr); if (!vars1) { - rc = 1; + rc = -1; goto err_exit; } @@ -835,7 +835,7 @@ __virNWFilterInstantiateFilter(bool tear vars = virNWFilterCreateVarsFrom(vars1, filterparams); if (!vars) { - rc = 1; + rc = -1; goto err_exit_vars1; } @@ -955,7 +955,7 @@ virNWFilterInstantiateFilterLate(const c driver, true, &foundNewFilter); - if (rc) { + if (rc < 0) { /* something went wrong... 'DOWN' the interface */ if ((virNetDevValidateConfig(ifname, NULL, ifindex) <= 0) || (virNetDevSetOnline(ifname, false) < 0)) { @@ -1012,7 +1012,7 @@ int virNWFilterRollbackUpdateFilter(cons _("Could not get access to ACL tech " "driver '%s'"), drvname); - return 1; + return -1; } /* don't tear anything while the address is being learned */ @@ -1038,7 +1038,7 @@ virNWFilterTearOldFilter(virDomainNetDef _("Could not get access to ACL tech " "driver '%s'"), drvname); - return 1; + return -1; } /* don't tear anything while the address is being learned */ @@ -1063,13 +1063,13 @@ _virNWFilterTeardownFilter(const char *i _("Could not get access to ACL tech " "driver '%s'"), drvname); - return 1; + return -1; } virNWFilterTerminateLearnReq(ifname); - if (virNWFilterLockIface(ifname)) - return 1; + if (virNWFilterLockIface(ifname) < 0) + return -1; techdriver->allTeardown(ifname); Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -149,7 +149,7 @@ virNWFilterLockIface(const char *ifname) goto err_exit; } - if (virMutexInitRecursive(&ifaceLock->lock)) { + if (virMutexInitRecursive(&ifaceLock->lock) < 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("mutex initialization failed")); VIR_FREE(ifaceLock); @@ -184,7 +184,7 @@ virNWFilterLockIface(const char *ifname) err_exit: virMutexUnlock(&ifaceMapLock); - return 1; + return -1; } @@ -248,7 +248,7 @@ virNWFilterRegisterLearnReq(virNWFilterI int virNWFilterTerminateLearnReq(const char *ifname) { - int rc = 1; + int rc = -1; int ifindex; virNWFilterIPAddrLearnReqPtr req; @@ -336,9 +336,6 @@ virNWFilterAddIpAddrForIfname(const char goto cleanup; } ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1); - /* FIXME: fix when return code of virNWFilterHashTablePut changes */ - if (ret) - ret = -1; goto cleanup; } else { if (virNWFilterVarValueAddValue(val, addr) < 0) @@ -494,7 +491,7 @@ learnIPAddressThread(void *arg) enum howDetect howDetected = 0; virNWFilterTechDriverPtr techdriver = req->techdriver; - if (virNWFilterLockIface(req->ifname)) + if (virNWFilterLockIface(req->ifname) < 0) goto err_no_lock; req->status = 0; @@ -520,7 +517,7 @@ learnIPAddressThread(void *arg) case DETECT_DHCP: if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, - NULL, false)) { + NULL, false) < 0) { req->status = EINVAL; goto done; } @@ -530,7 +527,7 @@ learnIPAddressThread(void *arg) break; default: if (techdriver->applyBasicRules(req->ifname, - req->macaddr)) { + req->macaddr) < 0) { req->status = EINVAL; goto done; } @@ -701,7 +698,7 @@ learnIPAddressThread(void *arg) sa.data.inet4.sin_addr.s_addr = vmaddr; char *inetaddr; - if ((inetaddr = virSocketAddrFormat(&sa))!= NULL) { + if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) { if (virNWFilterAddIpAddrForIfname(req->ifname, inetaddr) < 0) { VIR_ERROR(_("Failed to add IP address %s to IP address " "cache for interface %s"), inetaddr, req->ifname); @@ -781,14 +778,14 @@ virNWFilterLearnIPAddress(virNWFilterTec virNWFilterHashTablePtr ht = NULL; if (howDetect == 0) - return 1; + return -1; if ( !techdriver->canApplyBasicRules()) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("IP parameter must be provided since " "snooping the IP address does not work " "possibly due to missing tools")); - return 1; + return -1; } if (VIR_ALLOC(req) < 0) { @@ -802,7 +799,7 @@ virNWFilterLearnIPAddress(virNWFilterTec goto err_free_req; } - if (virNWFilterHashTablePutAll(filterparams, ht)) + if (virNWFilterHashTablePutAll(filterparams, ht) < 0) goto err_free_ht; req->filtername = strdup(filtername); @@ -838,7 +835,7 @@ virNWFilterLearnIPAddress(virNWFilterTec rc = virNWFilterRegisterLearnReq(req); - if (rc) + if (rc < 0) goto err_free_req; if (pthread_create(&req->thread, @@ -856,7 +853,7 @@ err_free_ht: err_free_req: virNWFilterIPAddrLearnReqFree(req); err_no_req: - return 1; + return -1; } #else @@ -876,7 +873,7 @@ virNWFilterLearnIPAddress(virNWFilterTec _("IP parameter must be given since libvirt " "was not compiled with IP address learning " "support")); - return 1; + return -1; } #endif /* HAVE_LIBPCAP */ @@ -895,35 +892,35 @@ virNWFilterLearnInit(void) { pendingLearnReq = virHashCreate(0, freeLearnReqEntry); if (!pendingLearnReq) { - return 1; + return -1; } - if (virMutexInit(&pendingLearnReqLock)) { + if (virMutexInit(&pendingLearnReqLock) < 0) { virNWFilterLearnShutdown(); - return 1; + return -1; } ipAddressMap = virNWFilterHashTableCreate(0); if (!ipAddressMap) { virReportOOMError(); virNWFilterLearnShutdown(); - return 1; + return -1; } - if (virMutexInit(&ipAddressMapLock)) { + if (virMutexInit(&ipAddressMapLock) < 0) { virNWFilterLearnShutdown(); - return 1; + return -1; } ifaceLockMap = virHashCreate(0, freeIfaceLock); if (!ifaceLockMap) { virNWFilterLearnShutdown(); - return 1; + return -1; } - if (virMutexInit(&ifaceMapLock)) { + if (virMutexInit(&ifaceMapLock) < 0) { virNWFilterLearnShutdown(); - return 1; + return -1; } return 0; Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -233,15 +233,15 @@ printVar(virNWFilterVarCombIterPtr vars, val = virNWFilterVarCombIterGetVarValue(vars, item->var); if (!val) { /* error has been reported */ - return 1; + return -1; } if (!virStrcpy(buf, val, bufsize)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - _("Buffer to small to print MAC address " + _("Buffer too small to print MAC address " "'%s' into"), item->var); - return 1; + return -1; } *done = 1; @@ -259,8 +259,8 @@ _printDataType(virNWFilterVarCombIterPtr int done; char *data; - if (printVar(vars, buf, bufsize, item, &done)) - return 1; + if (printVar(vars, buf, bufsize, item, &done) < 0) + return -1; if (done) return 0; @@ -269,12 +269,12 @@ _printDataType(virNWFilterVarCombIterPtr case DATATYPE_IPADDR: data = virSocketAddrFormat(&item->u.ipaddr); if (!data) - return 1; + return -1; if (snprintf(buf, bufsize, "%s", data) >= bufsize) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer too small for IP address")); VIR_FREE(data); - return 1; + return -1; } VIR_FREE(data); break; @@ -282,13 +282,13 @@ _printDataType(virNWFilterVarCombIterPtr case DATATYPE_IPV6ADDR: data = virSocketAddrFormat(&item->u.ipaddr); if (!data) - return 1; + return -1; if (snprintf(buf, bufsize, "%s", data) >= bufsize) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer too small for IPv6 address")); VIR_FREE(data); - return 1; + return -1; } VIR_FREE(data); break; @@ -298,7 +298,7 @@ _printDataType(virNWFilterVarCombIterPtr if (bufsize < VIR_MAC_STRING_BUFLEN) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer too small for MAC address")); - return 1; + return -1; } virFormatMacAddr(item->u.macaddr.addr, buf); @@ -310,7 +310,7 @@ _printDataType(virNWFilterVarCombIterPtr item->u.u8) >= bufsize) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer too small for uint8 type")); - return 1; + return -1; } break; @@ -320,7 +320,7 @@ _printDataType(virNWFilterVarCombIterPtr item->u.u32) >= bufsize) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer too small for uint32 type")); - return 1; + return -1; } break; @@ -330,7 +330,7 @@ _printDataType(virNWFilterVarCombIterPtr item->u.u16) >= bufsize) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer too small for uint16 type")); - return 1; + return -1; } break; @@ -340,14 +340,14 @@ _printDataType(virNWFilterVarCombIterPtr item->u.u8) >= bufsize) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer too small for uint8 type")); - return 1; + return -1; } break; default: virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("Unhandled datatype %x"), item->datatype); - return 1; + return -1; break; } @@ -417,7 +417,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns if (VIR_ALLOC(inst) < 0) { virReportOOMError(); - return 1; + return -1; } inst->commandTemplate = commandTemplate; @@ -442,7 +442,7 @@ ebtablesHandleEthHdr(virBufferPtr buf, if (HAS_ENTRY_ITEM(&ethHdr->dataSrcMACAddr)) { if (printDataType(vars, macaddr, sizeof(macaddr), - &ethHdr->dataSrcMACAddr)) + &ethHdr->dataSrcMACAddr) < 0) goto err_exit; virBufferAsprintf(buf, @@ -454,7 +454,7 @@ ebtablesHandleEthHdr(virBufferPtr buf, if (HAS_ENTRY_ITEM(&ethHdr->dataSrcMACMask)) { if (printDataType(vars, macaddr, sizeof(macaddr), - &ethHdr->dataSrcMACMask)) + &ethHdr->dataSrcMACMask) < 0) goto err_exit; virBufferAsprintf(buf, @@ -466,7 +466,7 @@ ebtablesHandleEthHdr(virBufferPtr buf, if (HAS_ENTRY_ITEM(&ethHdr->dataDstMACAddr)) { if (printDataType(vars, macaddr, sizeof(macaddr), - &ethHdr->dataDstMACAddr)) + &ethHdr->dataDstMACAddr) < 0) goto err_exit; virBufferAsprintf(buf, @@ -478,7 +478,7 @@ ebtablesHandleEthHdr(virBufferPtr buf, if (HAS_ENTRY_ITEM(&ethHdr->dataDstMACMask)) { if (printDataType(vars, macaddr, sizeof(macaddr), - &ethHdr->dataDstMACMask)) + &ethHdr->dataDstMACMask) < 0) goto err_exit; virBufferAsprintf(buf, @@ -492,7 +492,7 @@ ebtablesHandleEthHdr(virBufferPtr buf, err_exit: virBufferFreeAndReset(buf); - return 1; + return -1; } @@ -895,7 +895,7 @@ iptablesHandleSrcMacAddr(virBufferPtr bu if (printDataType(vars, macaddr, sizeof(macaddr), - srcMacAddr)) + srcMacAddr) < 0) goto err_exit; virBufferAsprintf(buf, @@ -909,7 +909,7 @@ iptablesHandleSrcMacAddr(virBufferPtr bu err_exit: virBufferFreeAndReset(buf); - return 1; + return -1; } @@ -940,7 +940,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, ipaddr, sizeof(ipaddr), - &ipHdr->dataSrcIPAddr)) + &ipHdr->dataSrcIPAddr) < 0) goto err_exit; virBufferAsprintf(buf, @@ -953,7 +953,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, number, sizeof(number), - &ipHdr->dataSrcIPMask)) + &ipHdr->dataSrcIPMask) < 0) goto err_exit; virBufferAsprintf(buf, @@ -964,7 +964,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, ipaddr, sizeof(ipaddr), - &ipHdr->dataSrcIPFrom)) + &ipHdr->dataSrcIPFrom) < 0) goto err_exit; virBufferAsprintf(buf, @@ -977,7 +977,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, ipaddr, sizeof(ipaddr), - &ipHdr->dataSrcIPTo)) + &ipHdr->dataSrcIPTo) < 0) goto err_exit; virBufferAsprintf(buf, @@ -990,7 +990,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, ipaddr, sizeof(ipaddr), - &ipHdr->dataDstIPAddr)) + &ipHdr->dataDstIPAddr) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1003,7 +1003,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, number, sizeof(number), - &ipHdr->dataDstIPMask)) + &ipHdr->dataDstIPMask) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1015,7 +1015,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, ipaddr, sizeof(ipaddr), - &ipHdr->dataDstIPFrom)) + &ipHdr->dataDstIPFrom) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1028,7 +1028,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, ipaddr, sizeof(ipaddr), - &ipHdr->dataDstIPTo)) + &ipHdr->dataDstIPTo) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1041,7 +1041,7 @@ iptablesHandleIpHdr(virBufferPtr buf, if (printDataType(vars, number, sizeof(number), - &ipHdr->dataDSCP)) + &ipHdr->dataDSCP) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1057,7 +1057,7 @@ iptablesHandleIpHdr(virBufferPtr buf, } else { if (printDataType(vars, number, sizeof(number), - &ipHdr->dataConnlimitAbove)) + &ipHdr->dataConnlimitAbove) < 0) goto err_exit; /* place connlimit after potential -m state --state ... @@ -1085,7 +1085,7 @@ err_exit: virBufferFreeAndReset(buf); virBufferFreeAndReset(afterStateMatch); - return 1; + return -1; } @@ -1106,7 +1106,7 @@ iptablesHandlePortData(virBufferPtr buf, if (HAS_ENTRY_ITEM(&portData->dataSrcPortStart)) { if (printDataType(vars, portstr, sizeof(portstr), - &portData->dataSrcPortStart)) + &portData->dataSrcPortStart) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1118,7 +1118,7 @@ iptablesHandlePortData(virBufferPtr buf, if (HAS_ENTRY_ITEM(&portData->dataSrcPortEnd)) { if (printDataType(vars, portstr, sizeof(portstr), - &portData->dataSrcPortEnd)) + &portData->dataSrcPortEnd) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1130,7 +1130,7 @@ iptablesHandlePortData(virBufferPtr buf, if (HAS_ENTRY_ITEM(&portData->dataDstPortStart)) { if (printDataType(vars, portstr, sizeof(portstr), - &portData->dataDstPortStart)) + &portData->dataDstPortStart) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1142,7 +1142,7 @@ iptablesHandlePortData(virBufferPtr buf, if (HAS_ENTRY_ITEM(&portData->dataDstPortEnd)) { if (printDataType(vars, portstr, sizeof(portstr), - &portData->dataDstPortEnd)) + &portData->dataDstPortEnd) < 0) goto err_exit; virBufferAsprintf(buf, @@ -1154,7 +1154,7 @@ iptablesHandlePortData(virBufferPtr buf, return 0; err_exit: - return 1; + return -1; } @@ -1244,7 +1244,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.tcpHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1253,7 +1253,7 @@ _iptablesCreateRuleInstance(int directio &rule->p.tcpHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.tcpHdrFilter.dataTCPFlags)) { @@ -1268,13 +1268,13 @@ _iptablesCreateRuleInstance(int directio if (iptablesHandlePortData(&buf, vars, &rule->p.tcpHdrFilter.portData, - directionIn)) + directionIn) < 0) goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.tcpHdrFilter.dataTCPOption)) { if (printDataType(vars, number, sizeof(number), - &rule->p.tcpHdrFilter.dataTCPOption)) + &rule->p.tcpHdrFilter.dataTCPOption) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -1299,7 +1299,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.udpHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1308,13 +1308,13 @@ _iptablesCreateRuleInstance(int directio &rule->p.udpHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; if (iptablesHandlePortData(&buf, vars, &rule->p.udpHdrFilter.portData, - directionIn)) + directionIn) < 0) goto err_exit; break; @@ -1332,7 +1332,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.udpliteHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1341,7 +1341,7 @@ _iptablesCreateRuleInstance(int directio &rule->p.udpliteHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; break; @@ -1360,7 +1360,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.espHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1369,7 +1369,7 @@ _iptablesCreateRuleInstance(int directio &rule->p.espHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; break; @@ -1388,7 +1388,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.ahHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1397,7 +1397,7 @@ _iptablesCreateRuleInstance(int directio &rule->p.ahHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; break; @@ -1416,7 +1416,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.sctpHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1425,13 +1425,13 @@ _iptablesCreateRuleInstance(int directio &rule->p.sctpHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; if (iptablesHandlePortData(&buf, vars, &rule->p.sctpHdrFilter.portData, - directionIn)) + directionIn) < 0) goto err_exit; break; @@ -1452,7 +1452,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.icmpHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1461,7 +1461,7 @@ _iptablesCreateRuleInstance(int directio &rule->p.icmpHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPType)) { @@ -1479,7 +1479,7 @@ _iptablesCreateRuleInstance(int directio if (printDataType(vars, number, sizeof(number), - &rule->p.icmpHdrFilter.dataICMPType)) + &rule->p.icmpHdrFilter.dataICMPType) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -1491,7 +1491,7 @@ _iptablesCreateRuleInstance(int directio if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPCode)) { if (printDataType(vars, number, sizeof(number), - &rule->p.icmpHdrFilter.dataICMPCode)) + &rule->p.icmpHdrFilter.dataICMPCode) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -1514,7 +1514,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.igmpHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1523,7 +1523,7 @@ _iptablesCreateRuleInstance(int directio &rule->p.igmpHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; break; @@ -1542,7 +1542,7 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.allHdrFilter.dataSrcMACAddr, directionIn, - &srcMacSkipped)) + &srcMacSkipped) < 0) goto err_exit; if (iptablesHandleIpHdr(&buf, @@ -1551,7 +1551,7 @@ _iptablesCreateRuleInstance(int directio &rule->p.allHdrFilter.ipHdr, directionIn, &skipRule, &skipMatch, - &prefix)) + &prefix) < 0) goto err_exit; break; @@ -1664,7 +1664,7 @@ printStateMatchFlags(int32_t flags, char if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); - return 1; + return -1; } *bufptr = virBufferContentAndReset(&buf); return 0; @@ -1704,8 +1704,8 @@ iptablesCreateRuleInstanceStateCtrl(virN } if (create && (rule->flags & IPTABLES_STATE_FLAGS)) { - if (printStateMatchFlags(rule->flags, &matchState)) - return 1; + if (printStateMatchFlags(rule->flags, &matchState) < 0) + return -1; } chainPrefix[1] = CHAINPREFIX_HOST_IN_TEMP; @@ -1723,7 +1723,7 @@ iptablesCreateRuleInstanceStateCtrl(virN maySkipICMP); VIR_FREE(matchState); - if (rc) + if (rc < 0) return rc; } @@ -1736,8 +1736,8 @@ iptablesCreateRuleInstanceStateCtrl(virN } if (create && (rule->flags & IPTABLES_STATE_FLAGS)) { - if (printStateMatchFlags(rule->flags, &matchState)) - return 1; + if (printStateMatchFlags(rule->flags, &matchState) < 0) + return -1; } chainPrefix[1] = CHAINPREFIX_HOST_OUT_TEMP; @@ -1756,7 +1756,7 @@ iptablesCreateRuleInstanceStateCtrl(virN VIR_FREE(matchState); - if (rc) + if (rc < 0) return rc; } @@ -1769,8 +1769,8 @@ iptablesCreateRuleInstanceStateCtrl(virN create = false; } else { if ((rule->flags & IPTABLES_STATE_FLAGS)) { - if (printStateMatchFlags(rule->flags, &matchState)) - return 1; + if (printStateMatchFlags(rule->flags, &matchState) < 0) + return -1; } } @@ -1852,7 +1852,7 @@ iptablesCreateRuleInstance(virNWFilterDe "RETURN", isIPv6, maySkipICMP); - if (rc) + if (rc < 0) return rc; @@ -1874,7 +1874,7 @@ iptablesCreateRuleInstance(virNWFilterDe "ACCEPT", isIPv6, maySkipICMP); - if (rc) + if (rc < 0) return rc; maySkipICMP = directionIn; @@ -1963,13 +1963,13 @@ ebtablesCreateRuleInstance(char chainPre if (ebtablesHandleEthHdr(&buf, vars, &rule->p.ethHdrFilter.ethHdr, - reverse)) + reverse) < 0) goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.ethHdrFilter.dataProtocolID)) { if (printDataTypeAsHex(vars, number, sizeof(number), - &rule->p.ethHdrFilter.dataProtocolID)) + &rule->p.ethHdrFilter.dataProtocolID) < 0) goto err_exit; virBufferAsprintf(&buf, " -p %s %s", @@ -1988,7 +1988,7 @@ ebtablesCreateRuleInstance(char chainPre if (ebtablesHandleEthHdr(&buf, vars, &rule->p.vlanHdrFilter.ethHdr, - reverse)) + reverse) < 0) goto err_exit; virBufferAddLit(&buf, @@ -1998,7 +1998,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM)) { \ if (printDataType(vars, \ field, sizeof(field), \ - &rule->p.STRUCT.ITEM)) \ + &rule->p.STRUCT.ITEM) < 0) \ goto err_exit; \ virBufferAsprintf(&buf, \ " " CLI " %s %s", \ @@ -2010,7 +2010,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM)) { \ if (printDataType(vars, \ field, sizeof(field), \ - &rule->p.STRUCT.ITEM)) \ + &rule->p.STRUCT.ITEM) < 0) \ goto err_exit; \ virBufferAsprintf(&buf, \ " " CLI " %s %s", \ @@ -2019,7 +2019,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.STRUCT.ITEM_HI)) { \ if (printDataType(vars, \ field, sizeof(field), \ - &rule->p.STRUCT.ITEM_HI)) \ + &rule->p.STRUCT.ITEM_HI) < 0) \ goto err_exit; \ virBufferAsprintf(&buf, SEP "%s", field); \ } \ @@ -2055,7 +2055,7 @@ ebtablesCreateRuleInstance(char chainPre if (ebtablesHandleEthHdr(&buf, vars, &rule->p.stpHdrFilter.ethHdr, - reverse)) + reverse) < 0) goto err_exit; virBufferAddLit(&buf, " -d " NWFILTER_MAC_BGA); @@ -2092,7 +2092,7 @@ ebtablesCreateRuleInstance(char chainPre if (ebtablesHandleEthHdr(&buf, vars, &rule->p.arpHdrFilter.ethHdr, - reverse)) + reverse) < 0) goto err_exit; virBufferAsprintf(&buf, " -p 0x%x", @@ -2103,7 +2103,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataHWType)) { if (printDataType(vars, number, sizeof(number), - &rule->p.arpHdrFilter.dataHWType)) + &rule->p.arpHdrFilter.dataHWType) < 0) goto err_exit; virBufferAsprintf(&buf, " --arp-htype %s %s", @@ -2114,7 +2114,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataOpcode)) { if (printDataType(vars, number, sizeof(number), - &rule->p.arpHdrFilter.dataOpcode)) + &rule->p.arpHdrFilter.dataOpcode) < 0) goto err_exit; virBufferAsprintf(&buf, " --arp-opcode %s %s", @@ -2125,7 +2125,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataProtocolType)) { if (printDataTypeAsHex(vars, number, sizeof(number), - &rule->p.arpHdrFilter.dataProtocolType)) + &rule->p.arpHdrFilter.dataProtocolType) < 0) goto err_exit; virBufferAsprintf(&buf, " --arp-ptype %s %s", @@ -2136,7 +2136,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataARPSrcIPAddr)) { if (printDataType(vars, ipaddr, sizeof(ipaddr), - &rule->p.arpHdrFilter.dataARPSrcIPAddr)) + &rule->p.arpHdrFilter.dataARPSrcIPAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2149,7 +2149,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataARPDstIPAddr)) { if (printDataType(vars, ipaddr, sizeof(ipaddr), - &rule->p.arpHdrFilter.dataARPDstIPAddr)) + &rule->p.arpHdrFilter.dataARPDstIPAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2162,7 +2162,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataARPSrcMACAddr)) { if (printDataType(vars, macaddr, sizeof(macaddr), - &rule->p.arpHdrFilter.dataARPSrcMACAddr)) + &rule->p.arpHdrFilter.dataARPSrcMACAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2175,7 +2175,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataARPDstMACAddr)) { if (printDataType(vars, macaddr, sizeof(macaddr), - &rule->p.arpHdrFilter.dataARPDstMACAddr)) + &rule->p.arpHdrFilter.dataARPDstMACAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2201,7 +2201,7 @@ ebtablesCreateRuleInstance(char chainPre if (ebtablesHandleEthHdr(&buf, vars, &rule->p.ipHdrFilter.ethHdr, - reverse)) + reverse) < 0) goto err_exit; virBufferAddLit(&buf, @@ -2210,7 +2210,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr)) { if (printDataType(vars, ipaddr, sizeof(ipaddr), - &rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr)) + &rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2222,7 +2222,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.ipHdr.dataSrcIPMask)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipHdrFilter.ipHdr.dataSrcIPMask)) + &rule->p.ipHdrFilter.ipHdr.dataSrcIPMask) + < 0) goto err_exit; virBufferAsprintf(&buf, "/%s", @@ -2234,7 +2235,7 @@ ebtablesCreateRuleInstance(char chainPre if (printDataType(vars, ipaddr, sizeof(ipaddr), - &rule->p.ipHdrFilter.ipHdr.dataDstIPAddr)) + &rule->p.ipHdrFilter.ipHdr.dataDstIPAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2246,7 +2247,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.ipHdr.dataDstIPMask)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipHdrFilter.ipHdr.dataDstIPMask)) + &rule->p.ipHdrFilter.ipHdr.dataDstIPMask) + < 0) goto err_exit; virBufferAsprintf(&buf, "/%s", @@ -2257,7 +2259,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.ipHdr.dataProtocolID)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipHdrFilter.ipHdr.dataProtocolID)) + &rule->p.ipHdrFilter.ipHdr.dataProtocolID) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2270,7 +2272,8 @@ ebtablesCreateRuleInstance(char chainPre if (printDataType(vars, number, sizeof(number), - &rule->p.ipHdrFilter.portData.dataSrcPortStart)) + &rule->p.ipHdrFilter.portData.dataSrcPortStart) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2282,7 +2285,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.portData.dataSrcPortEnd)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipHdrFilter.portData.dataSrcPortEnd)) + &rule->p.ipHdrFilter.portData.dataSrcPortEnd) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2295,7 +2299,8 @@ ebtablesCreateRuleInstance(char chainPre if (printDataType(vars, number, sizeof(number), - &rule->p.ipHdrFilter.portData.dataDstPortStart)) + &rule->p.ipHdrFilter.portData.dataDstPortStart) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2307,7 +2312,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.portData.dataDstPortEnd)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipHdrFilter.portData.dataDstPortEnd)) + &rule->p.ipHdrFilter.portData.dataDstPortEnd) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2319,7 +2325,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.ipHdr.dataDSCP)) { if (printDataTypeAsHex(vars, number, sizeof(number), - &rule->p.ipHdrFilter.ipHdr.dataDSCP)) + &rule->p.ipHdrFilter.ipHdr.dataDSCP) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2337,7 +2343,7 @@ ebtablesCreateRuleInstance(char chainPre if (ebtablesHandleEthHdr(&buf, vars, &rule->p.ipv6HdrFilter.ethHdr, - reverse)) + reverse) < 0) goto err_exit; virBufferAddLit(&buf, @@ -2346,7 +2352,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.ipHdr.dataSrcIPAddr)) { if (printDataType(vars, ipv6addr, sizeof(ipv6addr), - &rule->p.ipv6HdrFilter.ipHdr.dataSrcIPAddr)) + &rule->p.ipv6HdrFilter.ipHdr.dataSrcIPAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2358,7 +2364,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.ipHdr.dataSrcIPMask)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipv6HdrFilter.ipHdr.dataSrcIPMask)) + &rule->p.ipv6HdrFilter.ipHdr.dataSrcIPMask) + < 0) goto err_exit; virBufferAsprintf(&buf, "/%s", @@ -2370,7 +2377,7 @@ ebtablesCreateRuleInstance(char chainPre if (printDataType(vars, ipv6addr, sizeof(ipv6addr), - &rule->p.ipv6HdrFilter.ipHdr.dataDstIPAddr)) + &rule->p.ipv6HdrFilter.ipHdr.dataDstIPAddr) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2382,7 +2389,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.ipHdr.dataDstIPMask)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipv6HdrFilter.ipHdr.dataDstIPMask)) + &rule->p.ipv6HdrFilter.ipHdr.dataDstIPMask) + < 0) goto err_exit; virBufferAsprintf(&buf, "/%s", @@ -2393,7 +2401,7 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.ipHdr.dataProtocolID)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipv6HdrFilter.ipHdr.dataProtocolID)) + &rule->p.ipv6HdrFilter.ipHdr.dataProtocolID) < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2406,7 +2414,8 @@ ebtablesCreateRuleInstance(char chainPre if (printDataType(vars, number, sizeof(number), - &rule->p.ipv6HdrFilter.portData.dataSrcPortStart)) + &rule->p.ipv6HdrFilter.portData.dataSrcPortStart) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2418,7 +2427,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.portData.dataSrcPortEnd)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipv6HdrFilter.portData.dataSrcPortEnd)) + &rule->p.ipv6HdrFilter.portData.dataSrcPortEnd) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2431,7 +2441,8 @@ ebtablesCreateRuleInstance(char chainPre if (printDataType(vars, number, sizeof(number), - &rule->p.ipv6HdrFilter.portData.dataDstPortStart)) + &rule->p.ipv6HdrFilter.portData.dataDstPortStart) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2443,7 +2454,8 @@ ebtablesCreateRuleInstance(char chainPre if (HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.portData.dataDstPortEnd)) { if (printDataType(vars, number, sizeof(number), - &rule->p.ipv6HdrFilter.portData.dataDstPortEnd)) + &rule->p.ipv6HdrFilter.portData.dataDstPortEnd) + < 0) goto err_exit; virBufferAsprintf(&buf, @@ -2510,7 +2522,7 @@ err_exit: * Convert a single rule into its representation for later instantiation * * Returns 0 in case of success with the result stored in the data structure - * pointed to by res, != 0 otherwise. + * pointed to by res, -1 otherwise */ static int ebiptablesCreateRuleInstance(enum virDomainNetType nettype ATTRIBUTE_UNUSED, @@ -2542,7 +2554,7 @@ ebiptablesCreateRuleInstance(enum virDom vars, res, rule->tt == VIR_NWFILTER_RULE_DIRECTION_INOUT); - if (rc) + if (rc < 0) return rc; } @@ -2596,7 +2608,7 @@ ebiptablesCreateRuleInstance(enum virDom case VIR_NWFILTER_RULE_PROTOCOL_LAST: virNWFilterReportError(VIR_ERR_OPERATION_FAILED, "%s", _("illegal protocol type")); - rc = 1; + rc = -1; break; } @@ -2621,7 +2633,7 @@ ebiptablesCreateRuleInstanceIterate( */ vciter = virNWFilterVarCombIterCreate(vars, rule->vars, rule->nvars); if (!vciter) - return 1; + return -1; do { rc = ebiptablesCreateRuleInstance(nettype, @@ -2630,7 +2642,7 @@ ebiptablesCreateRuleInstanceIterate( ifname, vciter, res); - if (rc) + if (rc < 0) break; vciter = virNWFilterVarCombIterNext(vciter); } while (vciter != NULL); @@ -3111,7 +3123,7 @@ ebtablesApplyBasicRules(const char *ifna virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot create rules since ebtables tool is " "missing.")); - return 1; + return -1; } virFormatMacAddr(macaddr, macaddr_str); @@ -3170,7 +3182,7 @@ tear_down_tmpebchains: "%s", _("Some rules could not be created.")); - return 1; + return -1; } @@ -3186,7 +3198,7 @@ tear_down_tmpebchains: * names (true) or also perform the renaming to their final names as * part of this call (false) * - * Returns 0 on success, 1 on failure with the rules removed + * Returns 0 on success, -1 on failure with the rules removed * * Apply filtering rules so that the VM can only send and receive * DHCP traffic and nothing else. @@ -3207,13 +3219,15 @@ ebtablesApplyDHCPOnlyRules(const char *i virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot create rules since ebtables tool is " "missing.")); - return 1; + return -1; } if (dhcpserver) { virBufferAsprintf(&buf, " --ip-src %s", dhcpserver); - if (virBufferError(&buf)) - return 1; + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + return -1; + } srcIPParam = virBufferContentAndReset(&buf); } @@ -3298,7 +3312,7 @@ tear_down_tmpebchains: VIR_FREE(srcIPParam); - return 1; + return -1; } @@ -3307,7 +3321,7 @@ tear_down_tmpebchains: * * @ifname: name of the backend-interface to which to apply the rules * - * Returns 0 on success, 1 on failure with the rules removed + * Returns 0 on success, -1 on failure with the rules removed * * Apply filtering rules so that the VM cannot receive or send traffic. */ @@ -3322,7 +3336,7 @@ ebtablesApplyDropAllRules(const char *if virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot create rules since ebtables tool is " "missing.")); - return 1; + return -1; } ebiptablesAllTeardown(ifname); @@ -3368,7 +3382,7 @@ tear_down_tmpebchains: "%s", _("Some rules could not be created.")); - return 1; + return -1; } @@ -3575,13 +3589,13 @@ ebiptablesApplyNewRules(const char *ifna const char *name = inst[i]->neededProtocolChain; if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) { if (virHashUpdateEntry(chains_in_set, name, - &inst[i]->chainPriority)) { + &inst[i]->chainPriority) < 0) { virReportOOMError(); goto exit_free_sets; } } else { if (virHashUpdateEntry(chains_out_set, name, - &inst[i]->chainPriority)) { + &inst[i]->chainPriority) < 0) { virReportOOMError(); goto exit_free_sets; } @@ -3606,9 +3620,9 @@ ebiptablesApplyNewRules(const char *ifna /* create needed chains */ if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1, - &ebtChains, &nEbtChains) || + &ebtChains, &nEbtChains) < 0 || ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0, - &ebtChains, &nEbtChains)) { + &ebtChains, &nEbtChains) < 0) { goto tear_down_tmpebchains; } @@ -3809,7 +3823,7 @@ exit_free_sets: VIR_FREE(errmsg); - return 1; + return -1; } @@ -3905,7 +3919,7 @@ ebiptablesTearOldRules(const char *ifnam * * Remove all rules one after the other * - * Return 0 on success, 1 if execution of one or more cleanup + * Return 0 on success, -1 if execution of one or more cleanup * commands failed. */ static int @@ -3927,14 +3941,14 @@ ebiptablesRemoveRules(const char *ifname 'D', -1, 0); - if (ebiptablesExecCLI(&buf, &cli_status, NULL)) + if (ebiptablesExecCLI(&buf, &cli_status, NULL) < 0) goto err_exit; if (cli_status) { virNWFilterReportError(VIR_ERR_BUILD_FIREWALL, "%s", _("error while executing CLI commands")); - rc = 1; + rc = -1; } err_exit: @@ -4022,8 +4036,8 @@ ebiptablesDriverInit(bool privileged) if (!privileged) return 0; - if (virMutexInit(&execCLIMutex)) - return EINVAL; + if (virMutexInit(&execCLIMutex) < 0) + return -EINVAL; gawk_cmd_path = virFindFileInPath("gawk"); grep_cmd_path = virFindFileInPath("grep"); @@ -4086,7 +4100,7 @@ ebiptablesDriverInit(bool privileged) _("firewall tools were not found or " "cannot be used")); ebiptablesDriverShutdown(); - return ENOTSUP; + return -ENOTSUP; } ebiptables_driver.flags = TECHDRV_FLAG_INITIALIZED; Index: libvirt-acl/src/uml/uml_conf.c =================================================================== --- libvirt-acl.orig/src/uml/uml_conf.c +++ libvirt-acl/src/uml/uml_conf.c @@ -143,7 +143,7 @@ umlConnectTapDevice(virConnectPtr conn, } if (net->filter) { - if (virDomainConfNWFilterInstantiate(conn, net)) { + if (virDomainConfNWFilterInstantiate(conn, net) < 0) { if (template_ifname) VIR_FREE(net->ifname); goto error; Index: libvirt-acl/src/qemu/qemu_process.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_process.c +++ libvirt-acl/src/qemu/qemu_process.c @@ -2321,7 +2321,7 @@ qemuProcessFiltersInstantiate(virConnect for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; if ((net->filter) && (net->ifname)) { - if (virDomainConfNWFilterInstantiate(conn, net)) { + if (virDomainConfNWFilterInstantiate(conn, net) < 0) { err = 1; break; } Index: libvirt-acl/src/qemu/qemu_command.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_command.c +++ libvirt-acl/src/qemu/qemu_command.c @@ -275,8 +275,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr if (tapfd >= 0) { if ((net->filter) && (net->ifname)) { - err = virDomainConfNWFilterInstantiate(conn, net); - if (err) + if (virDomainConfNWFilterInstantiate(conn, net) < 0); VIR_FORCE_CLOSE(tapfd); } }

2 4

[libvirt] [PATCH] Provide a helper method virDomainLiveHelperMethod
by Lei Li 09 Dec '11

09 Dec '11

This chunk of code below repeated in several functions, factor it into a helper method virDomainLiveHelperMethod to eliminate duplicated code based on Eric and Adam's suggestion. I have tested it for all the relevant APIs changed. isActive = virDomainObjIsActive(vm); if (flags == VIR_DOMAIN_AFFECT_CURRENT) { if (isActive) flags = VIR_DOMAIN_AFFECT_LIVE; else flags = VIR_DOMAIN_AFFECT_CONFIG; } if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (!vm->persistent) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot change persistent config of a transient domain")); goto endjob; } if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) goto endjob; } Signed-off-by: Lei Li <lilei(a)linux.vnet.ibm.com> --- src/conf/domain_conf.c | 47 ++++++++ src/conf/domain_conf.h | 7 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 288 ++++------------------------------------------ 4 files changed, 77 insertions(+), 266 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 75e51a0..12ea12d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1670,6 +1670,53 @@ virDomainObjGetPersistentDef(virCapsPtr caps, } /* + * Helper method for --current --live --config option, and check with + * whether domain is active or can get persistent domain configuration. + * + * Return 0 if success, also change the flags and get the persistent + * domain configuration if needed. Return -1 on error. + */ +int +virDomainLiveConfigHelperMethod(virCapsPtr caps, + virDomainObjPtr dom, + unsigned int *flags, + virDomainDefPtr *persistentDef) +{ + bool isActive; + int ret = 0; + + isActive = virDomainObjIsActive(dom); + + if (*flags == VIR_DOMAIN_AFFECT_CURRENT) { + if (isActive) + *flags = VIR_DOMAIN_AFFECT_LIVE; + else + *flags = VIR_DOMAIN_AFFECT_CONFIG; + } + + if (!isActive && (*flags & VIR_DOMAIN_AFFECT_LIVE)) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domain is not running")); + ret = -1; + } + + if (*flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (!dom->persistent) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot change persistent config of a transient domain")); + ret = -1; + } + if (!(*persistentDef = virDomainObjGetPersistentDef(caps, dom))) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Get persistent config failed")); + ret = -1; + } + } + + return ret; +} + +/* * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else * is either waiting for 'dom' or still using it diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d6ed898..3229a6f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1737,6 +1737,13 @@ int virDomainObjSetDefTransient(virCapsPtr caps, virDomainDefPtr virDomainObjGetPersistentDef(virCapsPtr caps, virDomainObjPtr domain); + +int +virDomainLiveConfigHelperMethod(virCapsPtr caps, + virDomainObjPtr dom, + unsigned int *flags, + virDomainDefPtr *persistentDef); + virDomainDefPtr virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 99a1099..5962d93 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -358,6 +358,7 @@ virDomainLifecycleCrashTypeFromString; virDomainLifecycleCrashTypeToString; virDomainLifecycleTypeFromString; virDomainLifecycleTypeToString; +virDomainLiveConfigHelperMethod; virDomainLoadAllConfigs; virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e5ed9a..9991383 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1822,12 +1822,6 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, isActive = virDomainObjIsActive(vm); - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } if (flags == VIR_DOMAIN_MEM_MAXIMUM) { if (isActive) flags = VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_MEM_MAXIMUM; @@ -1835,21 +1829,8 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, flags = VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_MEM_MAXIMUM; } - if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto endjob; - } - - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto endjob; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto endjob; - } if (flags & VIR_DOMAIN_MEM_MAXIMUM) { /* resize the maximum memory */ @@ -3271,7 +3252,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, const char * type; int max; int ret = -1; - bool isActive; bool maximum; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -3299,16 +3279,11 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - isActive = virDomainObjIsActive(vm); maximum = (flags & VIR_DOMAIN_VCPU_MAXIMUM) != 0; flags &= ~VIR_DOMAIN_VCPU_MAXIMUM; - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags |= VIR_DOMAIN_AFFECT_LIVE; - else - flags |= VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) + goto endjob; /* MAXIMUM cannot be mixed with LIVE. */ if (maximum && (flags & VIR_DOMAIN_AFFECT_LIVE)) { @@ -3317,18 +3292,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; - } - - if (!vm->persistent && (flags & VIR_DOMAIN_AFFECT_CONFIG)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto endjob; - } - if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unknown virt type in domain definition '%d'"), @@ -3353,9 +3316,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto endjob; - switch (flags) { case VIR_DOMAIN_AFFECT_CONFIG: if (maximum) { @@ -3414,7 +3374,6 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, int maxcpu, hostcpus; virNodeInfo nodeinfo; int ret = -1; - bool isActive; qemuDomainObjPrivatePtr priv; bool canResetting = true; int pcpu; @@ -3434,20 +3393,8 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } - - if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("a domain is inactive; can change only " - "persistent config")); + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto cleanup; - } priv = vm->privateData; @@ -3458,16 +3405,6 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - if (nodeGetInfo(dom->conn, &nodeinfo) < 0) goto cleanup; hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); @@ -3567,7 +3504,6 @@ qemudDomainGetVcpuPinInfo(virDomainPtr dom, virNodeInfo nodeinfo; virDomainDefPtr targetDef = NULL; int ret = -1; - bool isActive; int maxcpu, hostcpus, vcpu, pcpu; int n; virDomainVcpuPinDefPtr *vcpupin_list; @@ -3589,33 +3525,13 @@ qemudDomainGetVcpuPinInfo(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &targetDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } targetDef = vm->def; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot get persistent config of a transient domain")); - goto cleanup; - } - if (!(targetDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - /* Coverity didn't realize that targetDef must be set if we got here. */ sa_assert(targetDef); @@ -3760,7 +3676,6 @@ qemudDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) virDomainObjPtr vm; virDomainDefPtr def; int ret = -1; - bool active; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -3778,34 +3693,11 @@ qemudDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } - active = virDomainObjIsActive(vm); - - if ((flags & (VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG)) == 0) { - if (active) - flags |= VIR_DOMAIN_VCPU_LIVE; - else - flags |= VIR_DOMAIN_VCPU_CONFIG; - } - if ((flags & VIR_DOMAIN_AFFECT_LIVE) && (flags & VIR_DOMAIN_AFFECT_CONFIG)) { - qemuReportError(VIR_ERR_INVALID_ARG, - _("invalid flag combination: (0x%x)"), flags); - return -1; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &def) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!active) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain not active")); - goto cleanup; - } def = vm->def; - } else { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is transient")); - goto cleanup; - } - def = vm->newDef ? vm->newDef : vm->def; } ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus; @@ -6014,7 +5906,6 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; - bool isActive; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -6028,22 +5919,10 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't mounted")); goto cleanup; @@ -6056,16 +5935,6 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, } } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - ret = 0; if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < nparams; i++) { @@ -6220,7 +6089,6 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, unsigned int val; int ret = -1; int rc; - bool isActive; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -6247,22 +6115,10 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't mounted")); goto cleanup; @@ -6275,16 +6131,6 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, } } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < *nparams && i < QEMU_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = &params[i]; @@ -6440,7 +6286,6 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; - bool isActive; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -6455,22 +6300,10 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup memory controller is not mounted")); @@ -6484,16 +6317,6 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, } } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - ret = 0; for (i = 0; i < nparams; i++) { virTypedParameterPtr param = &params[i]; @@ -6598,7 +6421,6 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, unsigned long long val; int ret = -1; int rc; - bool isActive; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -6617,22 +6439,10 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup memory controller is not mounted")); @@ -6646,16 +6456,6 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, } } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - if ((*nparams) == 0) { /* Current number of memory parameters supported by cgroups */ *nparams = QEMU_NB_MEM_PARAM; @@ -11126,7 +10926,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, const char *device = NULL; int ret = -1; int i; - bool isActive; int idx = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -11151,33 +10950,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } - - if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is not running")); + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto endjob; - } - - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto endjob; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto endjob; - idx = virDomainDiskIndexByName(persistentDef, disk, true); - if (idx < 0) - goto endjob; - } for (i = 0; i < nparams; i++) { virTypedParameterPtr param = &params[i]; @@ -11238,7 +11012,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - sa_assert(persistentDef && idx >= 0); + sa_assert(persistentDef); + idx = virDomainDiskIndexByName(persistentDef, disk, true); + if (idx < 0) + goto endjob; persistentDef->disks[idx]->blkdeviotune = info; ret = virDomainSaveConfig(driver->configDir, persistentDef); if (ret < 0) { @@ -11276,7 +11053,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, const char *device = NULL; int ret = -1; int i; - bool isActive; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -11310,20 +11086,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } - - if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is not running")); + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) goto endjob; - } if (flags & VIR_DOMAIN_AFFECT_LIVE) { priv = vm->privateData; @@ -11335,14 +11099,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is transient")); - goto endjob; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto endjob; - int idx = virDomainDiskIndexByName(vm->def, disk, true); if (idx < 0) goto endjob; -- 1.7.1

2 1