- Devel - libvirt List Archives

[libvirt] [PATCH V3 0/3] libxl: domain destroy fixes
by Jim Fehlig 16 Apr '15

16 Apr '15

V3 of a small series to fix issues wrt domain destroy V1: https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html V2: https://www.redhat.com/archives/libvir-list/2015-April/msg00072.html In this version, patch 3 is changed a bit to provide a wrapper for unlocking, destroying, and locking the domain. Existing libxl_domain_destroy callers are changed to use the wrapper. Jim Fehlig (3): libxl: Move job acquisition in libxlDomainStart to callers libxl: acquire a job when destroying a domain libxl: drop virDomainObj lock when destroying a domain src/libxl/libxl_domain.c | 106 ++++++++++++++++++++++---------------------- src/libxl/libxl_domain.h | 8 ++-- src/libxl/libxl_driver.c | 81 ++++++++++++++++++++++++--------- src/libxl/libxl_migration.c | 4 +- 4 files changed, 118 insertions(+), 81 deletions(-) -- 1.8.4.5

3 7

[libvirt] [PATCH 0/5] libxl: improve xl config parsing
by Jim Fehlig 16 Apr '15

16 Apr '15

This series was inspired by Marek's and Chunyan's patches to add support for <kernel>, <initrd>, and <cmdline> in Xen HVM domain config https://www.redhat.com/archives/libvir-list/2015-March/msg00328.html https://www.redhat.com/archives/libvir-list/2014-September/msg01006.html Patches 1 and 2 are trivial prep for patch 3, which moves parsing and formatting of <os> config out of the common code and into the xl and xm specfic parsing/formatting code. For ease of review, identical copies of xen{Parse,Format}OS are made in xen_{xl,xm}.c. Changes to the xl parsing/formatting functions are made in patch 4 and 5. Changing the order in which config is parsed/formatted has the unfortunate side affect of requiring corresponding changes to the test config data files. Patch 3 shoulders that burden. Jim Fehlig (5): xenconfig: export xenConfigCopyString xenconfig: remove redunant parsing of device_model xenconfig: move <os> parsing/formating to config-specific files xenconfig: don't use "kernel" for hvmloader libxl: support HVM direct kernel boot src/libxl/libxl_conf.c | 9 ++ src/xenconfig/xen_common.c | 146 +---------------- src/xenconfig/xen_common.h | 5 + src/xenconfig/xen_xl.c | 178 +++++++++++++++++++++ src/xenconfig/xen_xm.c | 141 ++++++++++++++++ tests/testutilsxen.c | 8 +- .../test-fullvirt-direct-kernel-boot.cfg | 29 ++++ .../test-fullvirt-direct-kernel-boot.xml | 49 ++++++ tests/xlconfigdata/test-fullvirt-multiusb.cfg | 5 +- tests/xlconfigdata/test-new-disk.cfg | 5 +- tests/xlconfigdata/test-spice.cfg | 5 +- tests/xlconfigtest.c | 3 + tests/xmconfigdata/test-escape-paths.cfg | 6 +- tests/xmconfigdata/test-fullvirt-force-hpet.cfg | 6 +- tests/xmconfigdata/test-fullvirt-force-nohpet.cfg | 6 +- tests/xmconfigdata/test-fullvirt-localtime.cfg | 6 +- tests/xmconfigdata/test-fullvirt-net-ioemu.cfg | 6 +- tests/xmconfigdata/test-fullvirt-net-netfront.cfg | 6 +- tests/xmconfigdata/test-fullvirt-new-cdrom.cfg | 6 +- tests/xmconfigdata/test-fullvirt-old-cdrom.cfg | 6 +- tests/xmconfigdata/test-fullvirt-parallel-tcp.cfg | 6 +- .../test-fullvirt-serial-dev-2-ports.cfg | 6 +- .../test-fullvirt-serial-dev-2nd-port.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-file.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-null.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-pipe.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-pty.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-stdio.cfg | 6 +- .../test-fullvirt-serial-tcp-telnet.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-tcp.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-udp.cfg | 6 +- tests/xmconfigdata/test-fullvirt-serial-unix.cfg | 6 +- tests/xmconfigdata/test-fullvirt-sound.cfg | 6 +- tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 6 +- tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 6 +- tests/xmconfigdata/test-fullvirt-utc.cfg | 6 +- tests/xmconfigdata/test-no-source-cdrom.cfg | 6 +- tests/xmconfigdata/test-paravirt-net-e1000.cfg | 2 +- tests/xmconfigdata/test-paravirt-net-vifname.cfg | 2 +- .../test-paravirt-new-pvfb-vncdisplay.cfg | 2 +- tests/xmconfigdata/test-paravirt-new-pvfb.cfg | 2 +- .../test-paravirt-old-pvfb-vncdisplay.cfg | 2 +- tests/xmconfigdata/test-paravirt-old-pvfb.cfg | 2 +- tests/xmconfigdata/test-paravirt-vcpu.cfg | 2 +- tests/xmconfigdata/test-pci-devs.cfg | 6 +- 45 files changed, 511 insertions(+), 242 deletions(-) create mode 100644 tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml -- 1.8.4.5

2 7

[libvirt] [PATCH] util: set MAC address for VF via netlink message to PF + VF# when possible
by Laine Stump 16 Apr '15

16 Apr '15

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1113474 When we set the MAC address of a network device as a part of setting up macvtap "passthrough" mode (where the domain has an emulated netdev connected to a host macvtap device that has exclusive use of the physical device, and sets the device MAC address to match its own, i.e. "<interface type='direct'> <source mode='passthrough' .../>"), we use ioctl(SIOCSIFHWADDR) giving it the name of that device. This is true even if it is an SRIOV Virtual Function (VF). But, when we are setting the MAC address / vlan ID of a VF in preparation for "hostdev network" passthrough (this is where we set the MAC address and vlan id of the VF before detaching the host net driver and assigning the device to the domain with PCI passthrough, i.e. "<interface type='hostdev'>", we do the setting via a netlink RTM_SETLINK message for that VF's Physical Function (PF), telling it the VF# we want to change. This sets an "administratively changed MAC" flag for that VF in the PF's driver, and from that point on (until the PF's driver is reloaded) that VF's MAC address can't be changed. This means that if a VF is used for hostdev passthrough, it will have the admin flag set, and future attempts to use that VF for macvtap passthrough will fail. The solution to this problem is to check if a device being used for macvtap passthrough is actually a VF; if so, we use the netlink RTM_SETLINK message to the PF to set the VF's mac address instead of ioctl(SIOCSIFHWADDR); if not, behavior does not change from previously. There are three pieces to making this work: 1) virNetDevMacVLan(Create|Delete)WithVPortProfile() now call virNetDev(Replace|Restore)NetConfig() rather than virNetDev(Replace|Restore)MacAddress() (simply passing -1 for VF# and vlanid). 2) virNetDev(Replace|Restore)NetConfig() check to see if the device is a VF. If so, they find the PF's name and VF#, allowing them to call virNetDev(Replace|Restore)VfConfig(). 3) To prevent mixups when detaching a macvtap passthrough device that had been attached while running an older version of libvirt, virNetDevRestoreVfConfig() is potentially given the preserved name of the VF, and if the proper statefile for a VF can't be found in the stateDir (${stateDir}/${pfname}_vf${vfid}), virNetDevRestoreMacAddress() is called instead (which will look in the file named ${stateDir}/${vfname}). This problem has existed in every version of libvirt that has both macvtap passthrough and interface type='hostdev'. Fortunately people seem to use one or the other though. --- (I'm still doing some testing on this, but as long as this approach doesn't cause any other regressions (so far it doesn't appear to), this is the final form. So any ACK given will be contingent on the functional tests passing.) src/util/virnetdev.c | 66 ++++++++++++++++++++++++++++++++++++++++----- src/util/virnetdevmacvlan.c | 4 +-- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index a7903c3..a3f302f 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2243,7 +2243,8 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf, } static int -virNetDevRestoreVfConfig(const char *pflinkdev, int vf, +virNetDevRestoreVfConfig(const char *pflinkdev, + int vf, const char *vflinkdev, const char *stateDir) { int rc = -1; @@ -2258,6 +2259,17 @@ virNetDevRestoreVfConfig(const char *pflinkdev, int vf, stateDir, pflinkdev, vf) < 0) return rc; + if (vflinkdev && !virFileExists(path)) { + /* this VF's config may have been stored with + * virNetDevReplaceMacAddress while running an older version + * of libvirt. If so, the ${pf}_vf${id} file won't exist. In + * that case, try to restore using the older method with the + * VF's name directly. + */ + rc = virNetDevRestoreMacAddress(vflinkdev, stateDir); + goto cleanup; + } + if (virFileReadAll(path, 128, &fileData) < 0) goto cleanup; @@ -2311,11 +2323,31 @@ virNetDevReplaceNetConfig(const char *linkdev, int vf, const virMacAddr *macaddress, int vlanid, const char *stateDir) { + int ret = -1; + char *pfdevname = NULL; + + if (vf == -1 && virNetDevIsVirtualFunction(linkdev)) { + /* If this really *is* a VF and the caller just didn't know + * it, we should set the MAC address via PF+vf# instead of + * setting directly via VF, because the latter will be + * rejected any time after the former has been done. + */ + if (virNetDevGetPhysicalFunction(linkdev, &pfdevname) < 0) + goto cleanup; + if (virNetDevGetVirtualFunctionIndex(pfdevname, linkdev, &vf) < 0) + goto cleanup; + linkdev = pfdevname; + } + if (vf == -1) - return virNetDevReplaceMacAddress(linkdev, macaddress, stateDir); + ret = virNetDevReplaceMacAddress(linkdev, macaddress, stateDir); else - return virNetDevReplaceVfConfig(linkdev, vf, macaddress, vlanid, - stateDir); + ret = virNetDevReplaceVfConfig(linkdev, vf, macaddress, vlanid, + stateDir); + + cleanup: + VIR_FREE(pfdevname); + return ret; } /** @@ -2330,10 +2362,32 @@ virNetDevReplaceNetConfig(const char *linkdev, int vf, int virNetDevRestoreNetConfig(const char *linkdev, int vf, const char *stateDir) { + int ret = -1; + char *pfdevname = NULL; + const char *vfdevname = NULL; + + if (vf == -1 && virNetDevIsVirtualFunction(linkdev)) { + /* If this really *is* a VF and the caller just didn't know + * it, we should set the MAC address via PF+vf# instead of + * setting directly via VF, because the latter will be + * rejected any time after the former has been done. + */ + if (virNetDevGetPhysicalFunction(linkdev, &pfdevname) < 0) + goto cleanup; + if (virNetDevGetVirtualFunctionIndex(pfdevname, linkdev, &vf) < 0) + goto cleanup; + vfdevname = linkdev; + linkdev = pfdevname; + } + if (vf == -1) - return virNetDevRestoreMacAddress(linkdev, stateDir); + ret = virNetDevRestoreMacAddress(linkdev, stateDir); else - return virNetDevRestoreVfConfig(linkdev, vf, stateDir); + ret = virNetDevRestoreVfConfig(linkdev, vf, vfdevname, stateDir); + + cleanup: + VIR_FREE(pfdevname); + return ret; } #else /* defined(__linux__) && defined(HAVE_LIBNL) */ diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 5fd2097..ea628c0 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -779,7 +779,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, * emulate their switch in firmware. */ if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { - if (virNetDevReplaceMacAddress(linkdev, macaddress, stateDir) < 0) + if (virNetDevReplaceNetConfig(linkdev, -1, macaddress, -1, stateDir) < 0) return -1; } @@ -914,7 +914,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, int vf = -1; if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) - ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir)); + ignore_value(virNetDevRestoreNetConfig(linkdev, vf, stateDir)); if (ifname) { if (virNetDevVPortProfileDisassociate(ifname, -- 2.1.0

2 2

[libvirt] [PATCH] virbuffer: fix build on rhel-6
by Pavel Hrdina 16 Apr '15

16 Apr '15

On rhel-6 is broken gcc that reports this warning: util/virbuffer.c:500: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op] Move the pragma directive before function virBufferEscapeString because since commit aeb5262e this function uses 'strchr' too. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- Pushed as build-breaker fix. src/util/virbuffer.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 3d13c90..5b338f8 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -421,6 +421,15 @@ virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr) buf->use += count; } +/* Work around spurious strchr() diagnostics given by -Wlogical-op + * for gcc < 4.6. Doing it via a local pragma keeps the damage + * smaller than disabling it on the package level. Unfortunately, the + * affected GCCs don't allow diagnostic push/pop which would have + * further reduced the impact. */ +#if BROKEN_GCC_WLOGICALOP +# pragma GCC diagnostic ignored "-Wlogical-op" +#endif + /** * virBufferEscapeString: * @buf: the buffer to append to @@ -535,15 +544,6 @@ virBufferEscapeSexpr(virBufferPtr buf, virBufferEscape(buf, '\\', "\\'", format, str); } -/* Work around spurious strchr() diagnostics given by -Wlogical-op - * for gcc < 4.6. Doing it via a local pragma keeps the damage - * smaller than disabling it on the package level. Unfortunately, the - * affected GCCs don't allow diagnostic push/pop which would have - * further reduced the impact. */ -#if BROKEN_GCC_WLOGICALOP -# pragma GCC diagnostic ignored "-Wlogical-op" -#endif - /** * virBufferEscape: * @buf: the buffer to append to -- 2.0.5

1 0

[libvirt] [PATCH] nwfilter: Partly initialize driver even for non-privileged users
by Michal Privoznik 16 Apr '15

16 Apr '15

https://bugzilla.redhat.com/show_bug.cgi?id=1211436 This reverts commit b7829f959b33c6e32422222a9ed745c0da7dc696. The previous fix was not correct. Like everywhere else, a driver is a global variable allocated in stateInitialize function (or something similar for stateless drivers). Later, when a driver API is called, it's possible that the global variable is accessed and dereferenced. Now, some drivers require root privileges because they undertake some actions reserved only for the system admin (e.g. manipulating host firewall). And here's the trouble, the NWFilter state initializer exited too early when finding out it's running unprivileged, leaving the global NWFilter driver variable uninitialized. Any subsequent API call that tried to lock the driver resulted in dereferencing the driver and thus crash. On the other hand, in order to not resurrect the bug the original commit was fixing, Let's forbid the nwfilter define in session mode. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> Conflicts: src/nwfilter/nwfilter_driver.c: Context. Code changed a bit since 2013. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/nwfilter/nwfilter_driver.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 8e3db43..1a868b6 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -176,9 +176,6 @@ nwfilterStateInitialize(bool privileged, char *base = NULL; DBusConnection *sysbus = NULL; - if (!privileged) - return 0; - if (virDBusHasSystemBus() && !(sysbus = virDBusGetSystemBus())) return -1; @@ -193,6 +190,9 @@ nwfilterStateInitialize(bool privileged, driver->watchingFirewallD = (sysbus != NULL); driver->privileged = privileged; + if (!privileged) + return 0; + nwfilterDriverLock(); if (virNWFilterIPAddrMapInit() < 0) @@ -535,6 +535,12 @@ nwfilterDefineXML(virConnectPtr conn, virNWFilterObjPtr nwfilter = NULL; virNWFilterPtr ret = NULL; + if (!driver->privileged) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Can't define NWFilters in session mode")); + return NULL; + } + nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); -- 2.0.5

2 1

[libvirt] [PATCH] virNetSocketNewConnectUNIX: Don't unlink(NULL)
by Michal Privoznik 16 Apr '15

16 Apr '15

There is a possibility that we jump onto error label with @lockpath still initialized to NULL. Here, the @lockpath should be unlink()-ed, but passing there a NULL is not a good idea. Don't do that. In fact, we should call unlink() only if we created the lock file successfully. Reported-by: John Ferlan <jferlan(a)redhat.com> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/rpc/virnetsocket.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index b824285..a59e3e1 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -622,7 +622,7 @@ int virNetSocketNewConnectUNIX(const char *path, usleep(5000); } - if (lockfd) { + if (lockfd != -1) { unlink(lockpath); VIR_FORCE_CLOSE(lockfd); VIR_FREE(lockpath); @@ -640,12 +640,13 @@ int virNetSocketNewConnectUNIX(const char *path, return 0; error: - if (lockfd) + if (lockfd != -1) { unlink(lockpath); + VIR_FORCE_CLOSE(lockfd); + } VIR_FREE(lockpath); VIR_FREE(rundir); VIR_FORCE_CLOSE(fd); - VIR_FORCE_CLOSE(lockfd); return -1; } #else -- 2.0.5

2 1

[libvirt] [PATCH] lxc: move wireless PHYs to a network namespace
by Michal Privoznik 16 Apr '15

16 Apr '15

From: Lubomir Rintel <lkundrak(a)v3.sk> The 802.11 interfaces can not be moved by themselves, their Phy has to move too. If there are other interfaces, they have to move too -- hopefully it's not too confusing. This is a less-invasive alternative to defining a new hostdev type for PHYs. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virnetdev.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index a816e5d..21fa9d0 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -560,20 +560,53 @@ int virNetDevSetMTUFromDevice(const char *ifname, */ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) { - int rc; + int ret = -1; char *pid = NULL; + char *phy = NULL; + char *phy_path = NULL; + int len; + const char *argv[] = { "ip", "link", "set", ifname, "netns", NULL, NULL }; + const char *iwargv[] = { + "iw", "phy", NULL, "set", "netns", NULL, NULL + }; + if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1) return -1; argv[5] = pid; - rc = virRun(argv, NULL); + if (virRun(argv, NULL) < 0) + goto cleanup; + /* The 802.11 wireless devices only move together with their PHY. */ + if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") < 0) + goto cleanup; + + if ((len = virFileReadAllQuiet(phy_path, 1024, &phy) < 0)) { + if (errno == ENOENT) { + /* Okay, this is not a wireless card. Claim success. */ + ret = 0; + } + goto cleanup; + } + + /* Remove a line break. */ + phy[len - 1] = '\0'; + + iwargv[2] = phy; + iwargv[5] = pid; + if (virRun(iwargv, NULL) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(phy_path); + VIR_FREE(phy); VIR_FREE(pid); - return rc; + return ret; } #if defined(SIOCSIFNAME) && defined(HAVE_STRUCT_IFREQ) -- 2.0.5

2 1

[libvirt] unclear docs about scsi hostdev
by Olaf Hering 16 Apr '15

16 Apr '15

The scsi host example in https://libvirt.org/formatdomain.html does not make it clear what "<adapter name='scsi_host0'/>" refers to. It implies that any host #0 will be used. Since the index numbers for each host are dynamic an unexpected device may be used by libvirt. The reason I'm looking at this is to support vscsi in libxl. I have a change for xl(1) and want to add also libvirt support. The xend vscsi driver supported both H:C:T:L and /dev/whatever notation to describe the physical dom0 device. This was translated into H:C:T:L for the xenlinux backend driver. The new pvops backend driver understands "naa.wwn:lun" and "H:C:T:L" format. My libxl implementation will also translate /dev/whatever into "naa.wwn:lun". My change supports both xenlinux and pvops. I think its natural to refer to a /dev/disk/by-*/* name for pdev, if the SCSI device supports that. Looks like _virDomainHostdevSubsysSCSI->u.host is very limited in what it accepts as input. What should be done to accept both "/dev/whatever" and "naa.wwn:lun" as pdev? The <address/> part looks like the same format what the frontend driver understands: "host:channel:target:lun". Olaf

1 1

[libvirt] [PATCH] virCondWaitUntil: calculate timespec correctly
by Michael Chapman 16 Apr '15

16 Apr '15

ts.tv_nsec was off by a factor of 1000, making timeouts less than a second in the future often expiring immediately. Signed-off-by: Michael Chapman <mike(a)very.puzzling.org> --- src/util/virthread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virthread.c b/src/util/virthread.c index c2a9e7f..6c49515 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -164,7 +164,7 @@ int virCondWaitUntil(virCondPtr c, virMutexPtr m, unsigned long long whenms) struct timespec ts; ts.tv_sec = whenms / 1000; - ts.tv_nsec = (whenms % 1000) * 1000; + ts.tv_nsec = (whenms % 1000) * 1000000; if ((ret = pthread_cond_timedwait(&c->cond, &m->lock, &ts)) != 0) { errno = ret; -- 2.1.0

3 3

[libvirt] [PATCH] daemon: Prefix sysctl configuration file with numbers
by Jiri Denemark 16 Apr '15

16 Apr '15

Apparently, files in /usr/lib/sysctl.d are usually prefixed with numbers for easier ordering. Let's be consistent with this. I chose 60 for libvirtd so that it goes after 50-default.conf. https://bugzilla.redhat.com/show_bug.cgi?id=1084876 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- Notes: Actually, %config(noreplace) for a file in /usr/lib/sysctl.d looks somewhat wrong to me and I think we should fix that too. After all the files are usually placed in /usr/lib because they are not supposed to be modified by administrators. They should be able to override the settings in /etc/. Any opinion about this? daemon/Makefile.am | 4 ++-- libvirt.spec.in | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b95a79d..bceaeb2 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -341,10 +341,10 @@ if WITH_SYSCTL install-sysctl: $(MKDIR_P) $(DESTDIR)$(prefix)/lib/sysctl.d $(INSTALL_DATA) $(srcdir)/libvirtd.sysctl \ - $(DESTDIR)$(prefix)/lib/sysctl.d/libvirtd.conf + $(DESTDIR)$(prefix)/lib/sysctl.d/60-libvirtd.conf uninstall-sysctl: - rm -f $(DESTDIR)$(prefix)/lib/sysctl.d/libvirtd.conf + rm -f $(DESTDIR)$(prefix)/lib/sysctl.d/60-libvirtd.conf rmdir $(DESTDIR)$(prefix)/lib/sysctl.d || : else ! WITH_SYSCTL install-sysctl: diff --git a/libvirt.spec.in b/libvirt.spec.in index 4c11ab7..e08c9e7 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1609,7 +1609,7 @@ mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \ %endif %if 0%{?rhel} == 5 -rm -f $RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/libvirtd.conf +rm -f $RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/60-libvirtd.conf %endif %clean @@ -1897,7 +1897,7 @@ exit 0 %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf %config(noreplace) %{_sysconfdir}/libvirt/virtlockd.conf %if 0%{?fedora} || 0%{?rhel} >= 6 -%config(noreplace) %{_prefix}/lib/sysctl.d/libvirtd.conf +%config(noreplace) %{_prefix}/lib/sysctl.d/60-libvirtd.conf %endif %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd -- 2.3.5

3 3

[libvirt] [PATCHv3] qemu: monitor: Refactor and fix monitor checking
by Peter Krempa 16 Apr '15

16 Apr '15

Among all the monitor APIs some where checking if mon is NULL and some were not. Since it's possible to have mon equal to NULL in case a second call is attempted once entered the monitor. This requires that every single API checks for the monitor. This patch adds a macro that helps checking the state of the monitor and either refactors existing checking code to use the macro or adds it in case it was missing. --- Notes: Version 3: - fixed the check in qemuMonitorGetAllBlockStatsInfo by moving it before the allocation - added macros for all the return value combinations - removed 'mon' from existing VIR_DEBUG macros src/qemu/qemu_monitor.c | 1151 ++++++++++++----------------------------------- src/qemu/qemu_monitor.h | 18 +- 2 files changed, 292 insertions(+), 877 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1f95547..6d7562d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -98,6 +98,47 @@ struct _qemuMonitor { int logfd; }; +/** + * QEMU_CHECK_MONITOR_FULL: + * @mon: monitor pointer variable to check, evaluated multiple times, no parentheses + * @force_json: force JSON monitor, true or false + * @exit: statement that is used to exit the function + * + * This macro checks that the monitor is valid for given operation and exits + * the function if not. The macro also adds a debug statement regarding the + * monitor. + */ +#define QEMU_CHECK_MONITOR_FULL(mon, force_json, exit) \ + if (!mon) { \ + virReportError(VIR_ERR_INVALID_ARG, "%s", \ + _("monitor must not be NULL")); \ + exit; \ + } \ + VIR_DEBUG("mon:%p vm:%p json:%d fd:%d", mon, mon->vm, mon->json, mon->fd); \ + if (force_json && !mon->json) { \ + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", \ + _("JSON monitor is required")); \ + exit; \ + } + +/* Check monitor and return NULL on error */ +#define QEMU_CHECK_MONITOR_NULL(mon) \ + QEMU_CHECK_MONITOR_FULL(mon, false, return NULL) +#define QEMU_CHECK_MONITOR_JSON_NULL(mon) \ + QEMU_CHECK_MONITOR_FULL(mon, true, return NULL) + +/* Check monitor and return -1 on error */ +#define QEMU_CHECK_MONITOR(mon) \ + QEMU_CHECK_MONITOR_FULL(mon, false, return -1) +#define QEMU_CHECK_MONITOR_JSON(mon) \ + QEMU_CHECK_MONITOR_FULL(mon, true, return -1) + +/* Check monitor and jump to the provided label */ +#define QEMU_CHECK_MONITOR_GOTO(mon, label) \ + QEMU_CHECK_MONITOR_FULL(mon, false, goto label) +#define QEMU_CHECK_MONITOR_JSON_GOTO(mon, label) \ + QEMU_CHECK_MONITOR_FULL(mon, true, goto label) + static virClassPtr qemuMonitorClass; static void qemuMonitorDispose(void *obj); @@ -1188,6 +1229,8 @@ qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, int ret = -1; char *path = NULL; + QEMU_CHECK_MONITOR(mon); + if (mon->json) { ret = qemuMonitorFindObjectPath(mon, "/", videoName, &path); if (ret < 0) { @@ -1216,6 +1259,8 @@ qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, char *json_cmd = NULL; int ret = -1; + QEMU_CHECK_MONITOR(mon); + if (mon->json) { /* hack to avoid complicating each call to text monitor functions */ json_cmd = qemuMonitorUnescapeArg(cmd); @@ -1527,13 +1572,7 @@ qemuMonitorEmitSerialChange(qemuMonitorPtr mon, int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (!mon->json) return 0; @@ -1546,13 +1585,7 @@ int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONStartCPUs(mon, conn); @@ -1564,13 +1597,7 @@ qemuMonitorStartCPUs(qemuMonitorPtr mon, int qemuMonitorStopCPUs(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONStopCPUs(mon); @@ -1584,13 +1611,9 @@ qemuMonitorGetStatus(qemuMonitorPtr mon, bool *running, virDomainPausedReason *reason) { - VIR_DEBUG("mon=%p, running=%p, reason=%p", mon, running, reason); + VIR_DEBUG("running=%p, reason=%p", running, reason); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetStatus(mon, running, reason); @@ -1602,13 +1625,7 @@ qemuMonitorGetStatus(qemuMonitorPtr mon, int qemuMonitorSystemPowerdown(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSystemPowerdown(mon); @@ -1620,13 +1637,7 @@ qemuMonitorSystemPowerdown(qemuMonitorPtr mon) int qemuMonitorSystemReset(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSystemReset(mon); @@ -1639,13 +1650,7 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, int **pids) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetCPUInfo(mon, pids); @@ -1659,13 +1664,9 @@ qemuMonitorSetLink(qemuMonitorPtr mon, const char *name, virDomainNetInterfaceLinkState state) { - VIR_DEBUG("mon=%p, name=%s, state=%u", mon, name, state); + VIR_DEBUG("name=%s, state=%u", name, state); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSetLink(mon, name, state); @@ -1678,13 +1679,7 @@ int qemuMonitorGetVirtType(qemuMonitorPtr mon, int *virtType) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetVirtType(mon, virtType); @@ -1697,13 +1692,7 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, unsigned long long *currmem) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetBalloonInfo(mon, currmem); @@ -1717,13 +1706,9 @@ qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats) { - VIR_DEBUG("mon=%p stats=%p nstats=%u", mon, stats, nr_stats); + VIR_DEBUG("stats=%p nstats=%u", stats, nr_stats); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) { ignore_value(qemuMonitorFindBalloonObjectPath(mon, "/")); @@ -1810,13 +1795,7 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon) int ret; virHashTablePtr table; - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return NULL; - } + QEMU_CHECK_MONITOR_NULL(mon); if (!(table = virHashCreate(32, virHashValueFree))) return NULL; @@ -1871,7 +1850,9 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, bool backingChain) { int ret = -1; - VIR_DEBUG("mon=%p ret_stats=%p, backing=%d", mon, ret_stats, backingChain); + VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain); + + QEMU_CHECK_MONITOR(mon); if (!(*ret_stats = virHashCreate(10, virHashValueFree))) goto error; @@ -1908,13 +1889,9 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, virHashTablePtr stats, bool backingChain) { - VIR_DEBUG("mon=%p, stats=%p, backing=%d", mon, stats, backingChain); + VIR_DEBUG("stats=%p, backing=%d", stats, backingChain); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("block capacity/size info requires JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONBlockStatsUpdateCapacity(mon, stats, backingChain); } @@ -1925,7 +1902,9 @@ qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent) { - VIR_DEBUG("mon=%p, dev_name=%s", mon, dev_name); + VIR_DEBUG("dev_name=%s", dev_name); + + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetBlockExtent(mon, dev_name, extent); @@ -1939,7 +1918,9 @@ qemuMonitorBlockResize(qemuMonitorPtr mon, const char *device, unsigned long long size) { - VIR_DEBUG("mon=%p, device=%s size=%llu", mon, device, size); + VIR_DEBUG("device=%s size=%llu", device, size); + + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONBlockResize(mon, device, size); @@ -1952,13 +1933,9 @@ int qemuMonitorSetVNCPassword(qemuMonitorPtr mon, const char *password) { - VIR_DEBUG("mon=%p, password=%p", mon, password); + VIR_DEBUG("password=%p", password); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (!password) password = ""; @@ -1999,14 +1976,10 @@ qemuMonitorSetPassword(qemuMonitorPtr mon, if (!protocol) return -1; - VIR_DEBUG("mon=%p, protocol=%s, password=%p, action_if_connected=%s", - mon, protocol, password, action_if_connected); + VIR_DEBUG("protocol=%s, password=%p, action_if_connected=%s", + protocol, password, action_if_connected); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (!password) password = ""; @@ -2031,13 +2004,9 @@ qemuMonitorExpirePassword(qemuMonitorPtr mon, if (!protocol) return -1; - VIR_DEBUG("mon=%p, protocol=%s, expire_time=%s", mon, protocol, expire_time); + VIR_DEBUG("protocol=%s, expire_time=%s", protocol, expire_time); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (!expire_time) expire_time = "now"; @@ -2053,13 +2022,9 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon, unsigned long newmem) { - VIR_DEBUG("mon=%p newmem=%lu", mon, newmem); + VIR_DEBUG("newmem=%lu", newmem); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSetBalloon(mon, newmem); @@ -2071,13 +2036,9 @@ qemuMonitorSetBalloon(qemuMonitorPtr mon, int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, bool online) { - VIR_DEBUG("mon=%p cpu=%d online=%d", mon, cpu, online); + VIR_DEBUG("cpu=%d online=%d", cpu, online); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSetCPU(mon, cpu, online); @@ -2091,13 +2052,9 @@ qemuMonitorEjectMedia(qemuMonitorPtr mon, const char *dev_name, bool force) { - VIR_DEBUG("mon=%p dev_name=%s force=%d", mon, dev_name, force); + VIR_DEBUG("dev_name=%s force=%d", dev_name, force); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONEjectMedia(mon, dev_name, force); @@ -2112,14 +2069,9 @@ qemuMonitorChangeMedia(qemuMonitorPtr mon, const char *newmedia, const char *format) { - VIR_DEBUG("mon=%p dev_name=%s newmedia=%s format=%s", - mon, dev_name, newmedia, format); + VIR_DEBUG("dev_name=%s newmedia=%s format=%s", dev_name, newmedia, format); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONChangeMedia(mon, dev_name, newmedia, format); @@ -2134,14 +2086,9 @@ qemuMonitorSaveVirtualMemory(qemuMonitorPtr mon, size_t length, const char *path) { - VIR_DEBUG("mon=%p offset=%llu length=%zu path=%s", - mon, offset, length, path); + VIR_DEBUG("offset=%llu length=%zu path=%s", offset, length, path); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSaveVirtualMemory(mon, offset, length, path); @@ -2156,14 +2103,9 @@ qemuMonitorSavePhysicalMemory(qemuMonitorPtr mon, size_t length, const char *path) { - VIR_DEBUG("mon=%p offset=%llu length=%zu path=%s", - mon, offset, length, path); + VIR_DEBUG("offset=%llu length=%zu path=%s", offset, length, path); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSavePhysicalMemory(mon, offset, length, path); @@ -2176,13 +2118,9 @@ int qemuMonitorSetMigrationSpeed(qemuMonitorPtr mon, unsigned long bandwidth) { - VIR_DEBUG("mon=%p bandwidth=%lu", mon, bandwidth); + VIR_DEBUG("bandwidth=%lu", bandwidth); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (bandwidth > QEMU_DOMAIN_MIG_BANDWIDTH_MAX) { virReportError(VIR_ERR_OVERFLOW, @@ -2202,13 +2140,9 @@ int qemuMonitorSetMigrationDowntime(qemuMonitorPtr mon, unsigned long long downtime) { - VIR_DEBUG("mon=%p downtime=%llu", mon, downtime); + VIR_DEBUG("downtime=%llu", downtime); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSetMigrationDowntime(mon, downtime); @@ -2221,19 +2155,9 @@ int qemuMonitorGetMigrationCacheSize(qemuMonitorPtr mon, unsigned long long *cacheSize) { - VIR_DEBUG("mon=%p cacheSize=%p", mon, cacheSize); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("cacheSize=%p", cacheSize); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetMigrationCacheSize(mon, cacheSize); } @@ -2243,19 +2167,9 @@ int qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon, unsigned long long cacheSize) { - VIR_DEBUG("mon=%p cacheSize=%llu", mon, cacheSize); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("cacheSize=%llu", cacheSize); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONSetMigrationCacheSize(mon, cacheSize); } @@ -2265,13 +2179,7 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon, qemuMonitorMigrationStatusPtr status) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetMigrationStatus(mon, status); @@ -2284,21 +2192,9 @@ int qemuMonitorGetSpiceMigrationStatus(qemuMonitorPtr mon, bool *spice_migrated) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); - if (mon->json) { - return qemuMonitorJSONGetSpiceMigrationStatus(mon, spice_migrated); - } else { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + return qemuMonitorJSONGetSpiceMigrationStatus(mon, spice_migrated); } @@ -2308,13 +2204,9 @@ qemuMonitorMigrateToFd(qemuMonitorPtr mon, int fd) { int ret; - VIR_DEBUG("mon=%p fd=%d flags=%x", mon, fd, flags); + VIR_DEBUG("fd=%d flags=%x", fd, flags); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (qemuMonitorSendFileHandle(mon, "migrate", fd) < 0) return -1; @@ -2342,13 +2234,9 @@ qemuMonitorMigrateToHost(qemuMonitorPtr mon, { int ret; char *uri = NULL; - VIR_DEBUG("mon=%p hostname=%s port=%d flags=%x", mon, hostname, port, flags); + VIR_DEBUG("hostname=%s port=%d flags=%x", hostname, port, flags); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (virAsprintf(&uri, "%s:%s:%d", protocol, hostname, port) < 0) return -1; @@ -2371,13 +2259,9 @@ qemuMonitorMigrateToCommand(qemuMonitorPtr mon, char *argstr; char *dest = NULL; int ret = -1; - VIR_DEBUG("mon=%p argv=%p flags=%x", mon, argv, flags); + VIR_DEBUG("argv=%p flags=%x", argv, flags); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); argstr = virArgvToString(argv); if (!argstr) @@ -2410,14 +2294,10 @@ qemuMonitorMigrateToFile(qemuMonitorPtr mon, int ret = -1; char *safe_target = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - VIR_DEBUG("mon=%p argv=%p target=%s offset=%llu flags=%x", - mon, argv, target, offset, flags); + VIR_DEBUG("argv=%p target=%s offset=%llu flags=%x", + argv, target, offset, flags); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (offset % QEMU_MONITOR_MIGRATE_TO_FILE_BS) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2470,13 +2350,9 @@ qemuMonitorMigrateToUnix(qemuMonitorPtr mon, { char *dest = NULL; int ret = -1; - VIR_DEBUG("mon=%p, unixfile=%s flags=%x", mon, unixfile, flags); + VIR_DEBUG("unixfile=%s flags=%x", unixfile, flags); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (virAsprintf(&dest, "unix:%s", unixfile) < 0) return -1; @@ -2494,13 +2370,7 @@ qemuMonitorMigrateToUnix(qemuMonitorPtr mon, int qemuMonitorMigrateCancel(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONMigrateCancel(mon); @@ -2516,13 +2386,9 @@ int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability) { - VIR_DEBUG("mon=%p capability=%s", mon, capability); + VIR_DEBUG("capability=%s", capability); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); /* No capability is supported without JSON monitor */ if (!mon->json) @@ -2536,22 +2402,9 @@ int qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, const char *dumpformat) { int ret; - VIR_DEBUG("mon=%p fd=%d dumpformat=%s", mon, fd, dumpformat); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("fd=%d dumpformat=%s", fd, dumpformat); - if (!mon->json) { - /* We don't have qemuMonitorTextDump(), so we should check mon->json - * here. - */ - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("dump-guest-memory is not supported in text mode")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0) return -1; @@ -2575,8 +2428,10 @@ qemuMonitorGraphicsRelocate(qemuMonitorPtr mon, int tlsPort, const char *tlsSubject) { - VIR_DEBUG("mon=%p type=%d hostname=%s port=%d tlsPort=%d tlsSubject=%s", - mon, type, hostname, port, tlsPort, NULLSTR(tlsSubject)); + VIR_DEBUG("type=%d hostname=%s port=%d tlsPort=%d tlsSubject=%s", + type, hostname, port, tlsPort, NULLSTR(tlsSubject)); + + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGraphicsRelocate(mon, @@ -2599,13 +2454,9 @@ int qemuMonitorAddUSBDisk(qemuMonitorPtr mon, const char *path) { - VIR_DEBUG("mon=%p path=%s", mon, path); + VIR_DEBUG("path=%s", path); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAddUSBDisk(mon, path); @@ -2619,13 +2470,9 @@ qemuMonitorAddUSBDeviceExact(qemuMonitorPtr mon, int bus, int dev) { - VIR_DEBUG("mon=%p bus=%d dev=%d", mon, bus, dev); + VIR_DEBUG("bus=%d dev=%d", bus, dev); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAddUSBDeviceExact(mon, bus, dev); @@ -2639,13 +2486,9 @@ qemuMonitorAddUSBDeviceMatch(qemuMonitorPtr mon, int vendor, int product) { - VIR_DEBUG("mon=%p vendor=%d product=%d", mon, vendor, product); + VIR_DEBUG("vendor=%d product=%d", vendor, product); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAddUSBDeviceMatch(mon, vendor, product); @@ -2659,15 +2502,10 @@ qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon, virDevicePCIAddress *hostAddr, virDevicePCIAddress *guestAddr) { - VIR_DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d", - mon, hostAddr->domain, hostAddr->bus, hostAddr->slot, - hostAddr->function); + VIR_DEBUG("domain=%d bus=%d slot=%d function=%d", + hostAddr->domain, hostAddr->bus, hostAddr->slot, hostAddr->function); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAddPCIHostDevice(mon, hostAddr, guestAddr); @@ -2682,13 +2520,9 @@ qemuMonitorAddPCIDisk(qemuMonitorPtr mon, const char *bus, virDevicePCIAddress *guestAddr) { - VIR_DEBUG("mon=%p path=%s bus=%s", mon, path, bus); + VIR_DEBUG("path=%s bus=%s", path, bus); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAddPCIDisk(mon, path, bus, guestAddr); @@ -2702,13 +2536,9 @@ qemuMonitorAddPCINetwork(qemuMonitorPtr mon, const char *nicstr, virDevicePCIAddress *guestAddr) { - VIR_DEBUG("mon=%p nicstr=%s", mon, nicstr); + VIR_DEBUG("nicstr=%s", nicstr); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAddPCINetwork(mon, nicstr, guestAddr); @@ -2721,15 +2551,11 @@ int qemuMonitorRemovePCIDevice(qemuMonitorPtr mon, virDevicePCIAddress *guestAddr) { - VIR_DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d", - mon, guestAddr->domain, guestAddr->bus, - guestAddr->slot, guestAddr->function); + VIR_DEBUG("domain=%d bus=%d slot=%d function=%d", + guestAddr->domain, guestAddr->bus, guestAddr->slot, + guestAddr->function); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONRemovePCIDevice(mon, guestAddr); @@ -2743,13 +2569,9 @@ qemuMonitorSendFileHandle(qemuMonitorPtr mon, const char *fdname, int fd) { - VIR_DEBUG("mon=%p, fdname=%s fd=%d", mon, fdname, fd); + VIR_DEBUG("fdname=%s fd=%d", fdname, fd); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (fd < 0) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -2778,15 +2600,11 @@ qemuMonitorCloseFileHandle(qemuMonitorPtr mon, int ret = -1; virErrorPtr error; - VIR_DEBUG("mon=%p fdname=%s", mon, fdname); + VIR_DEBUG("fdname=%s", fdname); error = virSaveLastError(); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - goto cleanup; - } + QEMU_CHECK_MONITOR_GOTO(mon, cleanup); if (mon->json) ret = qemuMonitorJSONCloseFileHandle(mon, fdname); @@ -2808,13 +2626,9 @@ qemuMonitorCloseFileHandle(qemuMonitorPtr mon, int qemuMonitorAddFd(qemuMonitorPtr mon, int fdset, int fd, const char *name) { - VIR_DEBUG("mon=%p, fdset=%d, fd=%d, name=%s", mon, fdset, fd, NULLSTR(name)); + VIR_DEBUG("fdset=%d, fd=%d, name=%s", fdset, fd, NULLSTR(name)); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); if (fd < 0 || fdset < 0) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -2829,13 +2643,7 @@ qemuMonitorAddFd(qemuMonitorPtr mon, int fdset, int fd, const char *name) return -1; } - if (mon->json) { - return qemuMonitorJSONAddFd(mon, fdset, fd, name); - } else { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("add fd requires JSON monitor")); - return -1; - } + return qemuMonitorJSONAddFd(mon, fdset, fd, name); } @@ -2848,21 +2656,13 @@ qemuMonitorRemoveFd(qemuMonitorPtr mon, int fdset, int fd) int ret = -1; virErrorPtr error; - VIR_DEBUG("mon=%p, fdset=%d, fd=%d", mon, fdset, fd); + VIR_DEBUG("fdset=%d, fd=%d", fdset, fd); error = virSaveLastError(); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - goto cleanup; - } + QEMU_CHECK_MONITOR_JSON_GOTO(mon, cleanup); - if (mon->json) - ret = qemuMonitorJSONRemoveFd(mon, fdset, fd); - else - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("remove fd requires JSON monitor")); + ret = qemuMonitorJSONRemoveFd(mon, fdset, fd); cleanup: if (error) { @@ -2882,16 +2682,12 @@ qemuMonitorAddHostNetwork(qemuMonitorPtr mon, int ret = -1; size_t i = 0, j = 0; - VIR_DEBUG("mon=%p netstr=%s tapfd=%p tapfdName=%p tapfdSize=%d " + VIR_DEBUG("netstr=%s tapfd=%p tapfdName=%p tapfdSize=%d " "vhostfd=%p vhostfdName=%p vhostfdSize=%d", - mon, netstr, tapfd, tapfdName, tapfdSize, + netstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); for (i = 0; i < tapfdSize; i++) { if (qemuMonitorSendFileHandle(mon, tapfdName[i], tapfd[i]) < 0) @@ -2929,13 +2725,9 @@ qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int vlan, const char *netname) { - VIR_DEBUG("mon=%p netname=%s", mon, netname); + VIR_DEBUG("netname=%s", netname); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", @@ -2956,16 +2748,12 @@ qemuMonitorAddNetdev(qemuMonitorPtr mon, int ret = -1; size_t i = 0, j = 0; - VIR_DEBUG("mon=%p netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d" + VIR_DEBUG("netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d" "vhostfd=%p vhostfdName=%p vhostfdSize=%d", - mon, netdevstr, tapfd, tapfdName, tapfdSize, + netdevstr, tapfd, tapfdName, tapfdSize, vhostfd, vhostfdName, vhostfdSize); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); for (i = 0; i < tapfdSize; i++) { if (qemuMonitorSendFileHandle(mon, tapfdName[i], tapfd[i]) < 0) @@ -3001,13 +2789,9 @@ int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, const char *alias) { - VIR_DEBUG("mon=%p alias=%s", mon, alias); + VIR_DEBUG("alias=%s", alias); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONRemoveNetdev(mon, alias); @@ -3020,19 +2804,9 @@ int qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias, virNetDevRxFilterPtr *filter) { - VIR_DEBUG("mon=%p alias=%s filter=%p", mon, alias, filter); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("alias=%s filter=%p", alias, filter); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("query-rx-filter requires JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONQueryRxFilter(mon, alias, filter); } @@ -3056,13 +2830,9 @@ qemuMonitorGetChardevInfo(qemuMonitorPtr mon, int ret; virHashTablePtr info = NULL; - VIR_DEBUG("mon=%p retinfo=%p", mon, retinfo); + VIR_DEBUG("retinfo=%p", retinfo); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - goto error; - } + QEMU_CHECK_MONITOR_GOTO(mon, error); if (!(info = virHashCreate(10, qemuMonitorChardevInfoFree))) goto error; @@ -3090,13 +2860,9 @@ qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virDevicePCIAddress *guestAddr) { - VIR_DEBUG("mon=%p type=%s", mon, bus); + VIR_DEBUG("type=%s", bus); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAttachPCIDiskController(mon, bus, guestAddr); @@ -3111,21 +2877,11 @@ qemuMonitorAttachDrive(qemuMonitorPtr mon, virDevicePCIAddress *controllerAddr, virDomainDeviceDriveAddress *driveAddr) { - VIR_DEBUG("mon=%p drivestr=%s domain=%d bus=%d slot=%d function=%d", - mon, drivestr, controllerAddr->domain, controllerAddr->bus, + VIR_DEBUG("drivestr=%s domain=%d bus=%d slot=%d function=%d", + drivestr, controllerAddr->domain, controllerAddr->bus, controllerAddr->slot, controllerAddr->function); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor should be using AddDrive")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorTextAttachDrive(mon, drivestr, controllerAddr, driveAddr); } @@ -3135,13 +2891,9 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, qemuMonitorPCIAddress **addrs) { - VIR_DEBUG("mon=%p addrs=%p", mon, addrs); + VIR_DEBUG("addrs=%p", addrs); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetAllPCIAddresses(mon, addrs); @@ -3154,13 +2906,9 @@ int qemuMonitorDriveDel(qemuMonitorPtr mon, const char *drivestr) { - VIR_DEBUG("mon=%p drivestr=%s", mon, drivestr); + VIR_DEBUG("drivestr=%s", drivestr); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONDriveDel(mon, drivestr); @@ -3173,13 +2921,9 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon, const char *devalias) { - VIR_DEBUG("mon=%p devalias=%s", mon, devalias); + VIR_DEBUG("devalias=%s", devalias); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONDelDevice(mon, devalias); @@ -3194,15 +2938,10 @@ qemuMonitorAddDeviceWithFd(qemuMonitorPtr mon, int fd, const char *fdname) { - VIR_DEBUG("mon=%p device=%s fd=%d fdname=%s", mon, devicestr, fd, - NULLSTR(fdname)); + VIR_DEBUG("device=%s fd=%d fdname=%s", devicestr, fd, NULLSTR(fdname)); int ret; - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (fd >= 0 && qemuMonitorSendFileHandle(mon, fdname, fd) < 0) return -1; @@ -3245,31 +2984,25 @@ qemuMonitorAddObject(qemuMonitorPtr mon, const char *objalias, virJSONValuePtr props) { - VIR_DEBUG("mon=%p type=%s objalias=%s props=%p", - mon, type, objalias, props); + VIR_DEBUG("type=%s objalias=%s props=%p", type, objalias, props); - if (!mon->json) { - virJSONValueFree(props); - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("object adding requires JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON_GOTO(mon, error); return qemuMonitorJSONAddObject(mon, type, objalias, props); -} + + error: + virJSONValueFree(props); + return -1; +} int qemuMonitorDelObject(qemuMonitorPtr mon, const char *objalias) { - VIR_DEBUG("mon=%p objalias=%s", mon, objalias); + VIR_DEBUG("objalias=%s", objalias); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("object deletion requires JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONDelObject(mon, objalias); } @@ -3279,13 +3012,9 @@ int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr) { - VIR_DEBUG("mon=%p drive=%s", mon, drivestr); + VIR_DEBUG("drive=%s", drivestr); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONAddDrive(mon, drivestr); @@ -3299,13 +3028,9 @@ qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) { - VIR_DEBUG("mon=%p alias=%s passphrase=%p(value hidden)", mon, alias, passphrase); + VIR_DEBUG("alias=%s passphrase=%p(value hidden)", alias, passphrase); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSetDrivePassphrase(mon, alias, passphrase); @@ -3317,13 +3042,9 @@ qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name) { - VIR_DEBUG("mon=%p, name=%s", mon, name); + VIR_DEBUG("name=%s", name); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONCreateSnapshot(mon, name); @@ -3334,13 +3055,9 @@ qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name) int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name) { - VIR_DEBUG("mon=%p, name=%s", mon, name); + VIR_DEBUG("name=%s", name); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONLoadSnapshot(mon, name); @@ -3352,13 +3069,9 @@ qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name) int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name) { - VIR_DEBUG("mon=%p, name=%s", mon, name); + VIR_DEBUG("name=%s", name); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONDeleteSnapshot(mon, name); @@ -3375,19 +3088,10 @@ qemuMonitorDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions, const char *device, const char *file, const char *format, bool reuse) { - VIR_DEBUG("mon=%p, actions=%p, device=%s, file=%s, format=%s, reuse=%d", - mon, actions, device, file, format, reuse); + VIR_DEBUG("actions=%p, device=%s, file=%s, format=%s, reuse=%d", + actions, device, file, format, reuse); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("disk snapshot requires JSON monitor")); - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONDiskSnapshot(mon, actions, device, file, format, reuse); } @@ -3401,16 +3105,12 @@ qemuMonitorDriveMirror(qemuMonitorPtr mon, unsigned int granularity, unsigned long long buf_size, unsigned int flags) { - VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s, bandwidth=%lld, " + VIR_DEBUG("device=%s, file=%s, format=%s, bandwidth=%lld, " "granularity=%#x, buf_size=%lld, flags=%x", - mon, device, file, NULLSTR(format), bandwidth, granularity, + device, file, NULLSTR(format), bandwidth, granularity, buf_size, flags); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("drive-mirror requires JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONDriveMirror(mon, device, file, format, bandwidth, granularity, buf_size, flags); @@ -3421,12 +3121,9 @@ qemuMonitorDriveMirror(qemuMonitorPtr mon, int qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) { - VIR_DEBUG("mon=%p, actions=%p", mon, actions); + VIR_DEBUG("actions=%p", actions); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("transaction requires JSON monitor")); - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONTransaction(mon, actions); } @@ -3439,15 +3136,10 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device, const char *backingName, unsigned long long bandwidth) { - VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, backingName=%s, " - "bandwidth=%llu", - mon, device, top, base, NULLSTR(backingName), bandwidth); + VIR_DEBUG("device=%s, top=%s, base=%s, backingName=%s, bandwidth=%llu", + device, top, base, NULLSTR(backingName), bandwidth); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("block-commit requires JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONBlockCommit(mon, device, top, base, backingName, bandwidth); @@ -3458,7 +3150,7 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device, bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon) { - if (!mon->json) + if (!mon || !mon->json) return false; return qemuMonitorJSONBlockCommit(mon, "bogus", NULL, NULL, NULL, 0) == -2; @@ -3473,11 +3165,7 @@ qemuMonitorDiskNameLookup(qemuMonitorPtr mon, virStorageSourcePtr top, virStorageSourcePtr target) { - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return NULL; - } + QEMU_CHECK_MONITOR_JSON_NULL(mon); return qemuMonitorJSONDiskNameLookup(mon, device, top, target); } @@ -3488,13 +3176,9 @@ int qemuMonitorDrivePivot(qemuMonitorPtr mon, const char *device) { - VIR_DEBUG("mon=%p, device=%s", mon, device); + VIR_DEBUG("device=%s", device); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("drive pivot requires JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONDrivePivot(mon, device); } @@ -3506,7 +3190,9 @@ qemuMonitorArbitraryCommand(qemuMonitorPtr mon, char **reply, bool hmp) { - VIR_DEBUG("mon=%p, cmd=%s, reply=%p, hmp=%d", mon, cmd, reply, hmp); + VIR_DEBUG("cmd=%s, reply=%p, hmp=%d", cmd, reply, hmp); + + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONArbitraryCommand(mon, cmd, reply, hmp); @@ -3518,7 +3204,7 @@ qemuMonitorArbitraryCommand(qemuMonitorPtr mon, int qemuMonitorInjectNMI(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONInjectNMI(mon); @@ -3533,7 +3219,9 @@ qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int *keycodes, unsigned int nkeycodes) { - VIR_DEBUG("mon=%p, holdtime=%u, nkeycodes=%u", mon, holdtime, nkeycodes); + VIR_DEBUG("holdtime=%u, nkeycodes=%u", holdtime, nkeycodes); + + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes); @@ -3546,13 +3234,9 @@ int qemuMonitorScreendump(qemuMonitorPtr mon, const char *file) { - VIR_DEBUG("mon=%p, file=%s", mon, file); + VIR_DEBUG("file=%s", file); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONScreendump(mon, file); @@ -3570,16 +3254,10 @@ qemuMonitorBlockStream(qemuMonitorPtr mon, unsigned long long bandwidth, bool modern) { - VIR_DEBUG("mon=%p, device=%s, base=%s, backingName=%s, bandwidth=%lluB, " - "modern=%d", - mon, device, NULLSTR(base), NULLSTR(backingName), - bandwidth, modern); + VIR_DEBUG("device=%s, base=%s, backingName=%s, bandwidth=%lluB, modern=%d", + device, NULLSTR(base), NULLSTR(backingName), bandwidth, modern); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("block jobs require JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONBlockStream(mon, device, base, backingName, bandwidth, modern); @@ -3591,13 +3269,9 @@ qemuMonitorBlockJobCancel(qemuMonitorPtr mon, const char *device, bool modern) { - VIR_DEBUG("mon=%p, device=%s, modern=%d", mon, device, modern); + VIR_DEBUG("device=%s, modern=%d", device, modern); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("block jobs require JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONBlockJobCancel(mon, device, modern); } @@ -3609,14 +3283,9 @@ qemuMonitorBlockJobSetSpeed(qemuMonitorPtr mon, unsigned long long bandwidth, bool modern) { - VIR_DEBUG("mon=%p, device=%s, bandwidth=%lluB, modern=%d", - mon, device, bandwidth, modern); + VIR_DEBUG("device=%s, bandwidth=%lluB, modern=%d", device, bandwidth, modern); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("block jobs require JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONBlockJobSetSpeed(mon, device, bandwidth, modern); } @@ -3628,14 +3297,9 @@ qemuMonitorBlockJobInfo(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, unsigned long long *bandwidth) { - VIR_DEBUG("mon=%p, device=%s, info=%p, bandwidth=%p", - mon, device, info, bandwidth); + VIR_DEBUG("device=%s, info=%p, bandwidth=%p", device, info, bandwidth); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("block jobs require JSON monitor")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONBlockJobInfo(mon, device, info, bandwidth); } @@ -3647,7 +3311,9 @@ qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions) { - VIR_DEBUG("mon=%p, device=%p, info=%p", mon, device, info); + VIR_DEBUG("device=%p, info=%p", device, info); + + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONSetBlockIoThrottle(mon, device, info, supportMaxOptions); @@ -3662,7 +3328,9 @@ qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, virDomainBlockIoTuneInfoPtr reply, bool supportMaxOptions) { - VIR_DEBUG("mon=%p, device=%p, reply=%p", mon, device, reply); + VIR_DEBUG("device=%p, reply=%p", device, reply); + + QEMU_CHECK_MONITOR(mon); if (mon->json) return qemuMonitorJSONGetBlockIoThrottle(mon, device, reply, supportMaxOptions); @@ -3733,15 +3401,11 @@ qemuMonitorOpenGraphics(qemuMonitorPtr mon, const char *fdname, bool skipauth) { - VIR_DEBUG("mon=%p protocol=%s fd=%d fdname=%s skipauth=%d", - mon, protocol, fd, NULLSTR(fdname), skipauth); + VIR_DEBUG("protocol=%s fd=%d fdname=%s skipauth=%d", + protocol, fd, NULLSTR(fdname), skipauth); int ret; - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); if (qemuMonitorSendFileHandle(mon, fdname, fd) < 0) return -1; @@ -3763,19 +3427,7 @@ qemuMonitorOpenGraphics(qemuMonitorPtr mon, int qemuMonitorSystemWakeup(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONSystemWakeup(mon); } @@ -3788,20 +3440,10 @@ qemuMonitorGetVersion(qemuMonitorPtr mon, int *micro, char **package) { - VIR_DEBUG("mon=%p major=%p minor=%p micro=%p package=%p", - mon, major, minor, micro, package); + VIR_DEBUG("major=%p minor=%p micro=%p package=%p", + major, minor, micro, package); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetVersion(mon, major, minor, micro, package); } @@ -3811,19 +3453,9 @@ int qemuMonitorGetMachines(qemuMonitorPtr mon, qemuMonitorMachineInfoPtr **machines) { - VIR_DEBUG("mon=%p machines=%p", mon, machines); + VIR_DEBUG("machines=%p", machines); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetMachines(mon, machines); } @@ -3844,19 +3476,9 @@ int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, char ***cpus) { - VIR_DEBUG("mon=%p cpus=%p", mon, cpus); + VIR_DEBUG("cpus=%p", cpus); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetCPUDefinitions(mon, cpus); } @@ -3866,19 +3488,9 @@ int qemuMonitorGetCommands(qemuMonitorPtr mon, char ***commands) { - VIR_DEBUG("mon=%p commands=%p", mon, commands); + VIR_DEBUG("commands=%p", commands); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetCommands(mon, commands); } @@ -3888,19 +3500,9 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon, char ***events) { - VIR_DEBUG("mon=%p events=%p", mon, events); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("events=%p", events); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetEvents(mon, events); } @@ -3914,19 +3516,9 @@ qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon, char ***params, bool *found) { - VIR_DEBUG("mon=%p option=%s params=%p", mon, option, params); + VIR_DEBUG("option=%s params=%p", option, params); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetCommandLineOptionParameters(mon, option, params, found); @@ -3938,19 +3530,9 @@ qemuMonitorGetKVMState(qemuMonitorPtr mon, bool *enabled, bool *present) { - VIR_DEBUG("mon=%p enabled=%p present=%p", mon, enabled, present); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("enabled=%p present=%p", enabled, present); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetKVMState(mon, enabled, present); } @@ -3960,19 +3542,9 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, char ***types) { - VIR_DEBUG("mon=%p types=%p", mon, types); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("types=%p", types); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetObjectTypes(mon, types); } @@ -3983,19 +3555,9 @@ qemuMonitorGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props) { - VIR_DEBUG("mon=%p type=%s props=%p", mon, type, props); + VIR_DEBUG("type=%s props=%p", type, props); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetObjectProps(mon, type, props); } @@ -4004,19 +3566,7 @@ qemuMonitorGetObjectProps(qemuMonitorPtr mon, char * qemuMonitorGetTargetArch(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return NULL; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return NULL; - } + QEMU_CHECK_MONITOR_JSON_NULL(mon); return qemuMonitorJSONGetTargetArch(mon); } @@ -4026,13 +3576,7 @@ int qemuMonitorGetMigrationCapabilities(qemuMonitorPtr mon, char ***capabilities) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); /* No capability is supported without JSON monitor */ if (!mon->json) @@ -4049,13 +3593,9 @@ int qemuMonitorGetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability) { - VIR_DEBUG("mon=%p capability=%d", mon, capability); + VIR_DEBUG("capability=%d", capability); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); /* No capability is supported without JSON monitor */ if (!mon->json) @@ -4070,19 +3610,9 @@ qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability, bool state) { - VIR_DEBUG("mon=%p capability=%d", mon, capability); + VIR_DEBUG("capability=%d", capability); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONSetMigrationCapability(mon, capability, state); } @@ -4093,19 +3623,9 @@ qemuMonitorNBDServerStart(qemuMonitorPtr mon, const char *host, unsigned int port) { - VIR_DEBUG("mon=%p host=%s port=%u", mon, host, port); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("host=%s port=%u", host, port); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONNBDServerStart(mon, host, port); } @@ -4116,19 +3636,9 @@ qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable) { - VIR_DEBUG("mon=%p deviceID=%s", mon, deviceID); + VIR_DEBUG("deviceID=%s", deviceID); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONNBDServerAdd(mon, deviceID, writable); } @@ -4137,19 +3647,7 @@ qemuMonitorNBDServerAdd(qemuMonitorPtr mon, int qemuMonitorNBDServerStop(qemuMonitorPtr mon) { - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONNBDServerStop(mon); } @@ -4159,19 +3657,9 @@ int qemuMonitorGetTPMModels(qemuMonitorPtr mon, char ***tpmmodels) { - VIR_DEBUG("mon=%p tpmmodels=%p", mon, tpmmodels); + VIR_DEBUG("tpmmodels=%p", tpmmodels); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetTPMModels(mon, tpmmodels); } @@ -4181,19 +3669,9 @@ int qemuMonitorGetTPMTypes(qemuMonitorPtr mon, char ***tpmtypes) { - VIR_DEBUG("mon=%p tpmtypes=%p", mon, tpmtypes); + VIR_DEBUG("tpmtypes=%p", tpmtypes); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetTPMTypes(mon, tpmtypes); } @@ -4204,19 +3682,9 @@ qemuMonitorAttachCharDev(qemuMonitorPtr mon, const char *chrID, virDomainChrSourceDefPtr chr) { - VIR_DEBUG("mon=%p chrID=%s chr=%p", mon, chrID, chr); + VIR_DEBUG("chrID=%s chr=%p", chrID, chr); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONAttachCharDev(mon, chrID, chr); } @@ -4226,19 +3694,9 @@ int qemuMonitorDetachCharDev(qemuMonitorPtr mon, const char *chrID) { - VIR_DEBUG("mon=%p chrID=%s", mon, chrID); + VIR_DEBUG("chrID=%s", chrID); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONDetachCharDev(mon, chrID); } @@ -4248,19 +3706,9 @@ int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon, char ***aliases) { - VIR_DEBUG("mon=%p, aliases=%p", mon, aliases); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + VIR_DEBUG("aliases=%p", aliases); - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONGetDeviceAliases(mon, aliases); } @@ -4304,19 +3752,9 @@ qemuMonitorGetGuestCPU(qemuMonitorPtr mon, virArch arch, virCPUDataPtr *data) { - VIR_DEBUG("mon=%p, arch='%s' data='%p'", mon, virArchToString(arch), data); + VIR_DEBUG("arch='%s' data='%p'", virArchToString(arch), data); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); *data = NULL; @@ -4339,20 +3777,7 @@ qemuMonitorGetGuestCPU(qemuMonitorPtr mon, int qemuMonitorRTCResetReinjection(qemuMonitorPtr mon) { - - VIR_DEBUG("mon=%p", mon); - - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } - - if (!mon->json) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); return qemuMonitorJSONRTCResetReinjection(mon); } @@ -4374,13 +3799,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon, qemuMonitorIOThreadInfoPtr **iothreads) { - VIR_DEBUG("mon=%p iothreads=%p", mon, iothreads); + VIR_DEBUG("iothreads=%p", iothreads); - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR(mon); /* Requires JSON to make the query */ if (!mon->json) { @@ -4418,16 +3839,12 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, virHashTablePtr *info) { - VIR_DEBUG("mon=%p info=%p", mon, info); + VIR_DEBUG("info=%p", info); int ret; *info = NULL; - if (!mon) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); - return -1; - } + QEMU_CHECK_MONITOR_JSON(mon); if (!mon->json) return -2; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 3e9c43c..cd4cc66 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -712,7 +712,7 @@ int qemuMonitorDiskSnapshot(qemuMonitorPtr mon, const char *format, bool reuse); int qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2); int qemuMonitorDriveMirror(qemuMonitorPtr mon, const char *device, const char *file, @@ -721,10 +721,10 @@ int qemuMonitorDriveMirror(qemuMonitorPtr mon, unsigned int granularity, unsigned long long buf_size, unsigned int flags) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorDrivePivot(qemuMonitorPtr mon, const char *device) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2); int qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device, @@ -732,15 +732,13 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *base, const char *backingName, unsigned long long bandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon); char *qemuMonitorDiskNameLookup(qemuMonitorPtr mon, const char *device, virStorageSourcePtr top, virStorageSourcePtr target) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, @@ -763,12 +761,12 @@ int qemuMonitorBlockStream(qemuMonitorPtr mon, const char *backingName, unsigned long long bandwidth, bool modern) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2); int qemuMonitorBlockJobCancel(qemuMonitorPtr mon, const char *device, bool modern) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2); int qemuMonitorBlockJobSetSpeed(qemuMonitorPtr mon, const char *device, @@ -779,7 +777,7 @@ int qemuMonitorBlockJobInfo(qemuMonitorPtr mon, const char *device, virDomainBlockJobInfoPtr info, unsigned long long *bandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorOpenGraphics(qemuMonitorPtr mon, const char *protocol, -- 2.3.5

2 2

[libvirt] building xen and libvirt with odd --prefix fails
by Olaf Hering 16 Apr '15

16 Apr '15

My xen is configured with "--prefix=/odd/path --enable-rpath". My libvirt is configured with "env PKG_CONFIG_PATH=/odd/path/share/pkgconfig bash -x autogen.sh --without-xen --with-libxl". Now make in libvirt fails to find "xen/xen.h" needed by xenconfig/xen_common.c, I think it expects it in /usr/include. But I have no xen-devel.rpm installed. I wonder if there are wrong assumptions in libvirt about xen. Since a while xen has a xenlight.pc which contains the required info to find headers and libs. Can there ever be a case where libxl is somewhere else than libxenstore and whatever else is provided by xen? I think they always go into the very same place. So shouldnt libvirt be updated to first check for xenlight.pc, and use the result for everything related to xen? I think that would work for every xen version which has a xenlight.pc (4.5+). And initially I did not pass the matching --prefix to libvirts configure. Even with matching --prefix libvirt does not look in $prefix/include for xen/xen.h, even with --includedir=$prefix/include. Olaf

1 1

[libvirt] [PATCHv2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls
by Erik Skultety 16 Apr '15

16 Apr '15

This patch adds checks for empty bitmaps right after the calls of virBitmapParse. These only include spots where set API's are called and where domain's XML is parsed. Also, it partially reverts commit 983f5a which added a check for invalid nodeset "0,^0" into virBitmapParse function. This change broke the logic, as an empty bitmap should not cause an error. https://bugzilla.redhat.com/show_bug.cgi?id=1210545 --- src/conf/domain_conf.c | 35 +++++++++++++++++++++++++++++++---- src/conf/numa_conf.c | 23 +++++++++++++++++++---- src/qemu/qemu_driver.c | 5 +++-- src/util/virbitmap.c | 3 --- src/xenconfig/xen_sxpr.c | 7 +++++++ tests/virbitmaptest.c | 13 ++++++++++--- 6 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 823e003..f7f68ba 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11577,6 +11577,12 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, if (virBitmapParse(nodemask, 0, &def->sourceNodes, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; + + if (virBitmapIsAllClear(def->sourceNodes)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodemask': %s"), nodemask); + goto cleanup; + } } ret = 0; @@ -13265,6 +13271,13 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; + if (virBitmapIsAllClear(def->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'cpuset': %s"), + tmp); + goto error; + } + cleanup: VIR_FREE(tmp); ctxt->node = oldnode; @@ -13366,6 +13379,12 @@ virDomainHugepagesParseXML(xmlNodePtr node, if (virBitmapParse(nodeset, 0, &hugepage->nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; + + if (virBitmapIsAllClear(hugepage->nodemask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodeset': %s"), nodeset); + goto cleanup; + } } ret = 0; @@ -13487,13 +13506,14 @@ virDomainThreadSchedParse(xmlNodePtr node, goto error; } - if (!virBitmapParse(tmp, 0, &sp->ids, - VIR_DOMAIN_CPUMASK_LEN) || - virBitmapIsAllClear(sp->ids) || + if (virBitmapParse(tmp, 0, &sp->ids, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto error; + + if (virBitmapIsAllClear(sp->ids) || virBitmapNextSetBit(sp->ids, -1) < minid || virBitmapLastSetBit(sp->ids) > maxid) { - virReportError(VIR_ERR_XML_ERROR, + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid value of '%s': %s"), name, tmp); goto error; @@ -13861,6 +13881,13 @@ virDomainDefParseXML(xmlDocPtr xml, if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; + + if (virBitmapIsAllClear(def->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'cpuset': %s"), tmp); + goto error; + } + VIR_FREE(tmp); } } diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 8a0f686..7ad3f66 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -178,6 +178,12 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr numa, if (virBitmapParse(tmp, 0, &mem_node->nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; + + if (virBitmapIsAllClear(mem_node->nodeset)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodeset': %s"), tmp); + goto cleanup; + } VIR_FREE(tmp); } @@ -233,10 +239,19 @@ virDomainNumatuneParseXML(virDomainNumaPtr numa, } VIR_FREE(tmp); - if ((tmp = virXMLPropString(node, "nodeset")) && - virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; - VIR_FREE(tmp); + tmp = virXMLPropString(node, "nodeset"); + if (tmp) { + if (virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; + + if (virBitmapIsAllClear(nodeset)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodeset': %s"), tmp); + goto cleanup; + } + + VIR_FREE(tmp); + } } if (virDomainNumatuneSet(numa, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f37a11e..cbb6e1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10134,8 +10134,9 @@ qemuDomainSetNumaParameters(virDomainPtr dom, goto endjob; if (virBitmapIsAllClear(nodeset)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Invalid nodeset for numatune")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("Invalid nodeset of 'numatune': %s"), + param->value.s); goto endjob; } } diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 5322bce..bf905ab 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -416,9 +416,6 @@ virBitmapParse(const char *str, } } - if (virBitmapIsAllClear(*bitmap)) - goto error; - return virBitmapCountBits(*bitmap); error: diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 5a170d3..d77abf3 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1165,6 +1165,13 @@ xenParseSxpr(const struct sexpr *root, if (virBitmapParse(cpus, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; + + if (virBitmapIsAllClear(def->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'cpumask': %s"), + cpus); + goto error; + } } def->maxvcpus = sexpr_int(root, "domain/vcpus"); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index f247275..9a84e4c 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -524,16 +524,23 @@ static int test10(const void *opaque ATTRIBUTE_UNUSED) { int ret = -1; - virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL; + virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL; if (virBitmapParse("0-3,5-8,11-15", 0, &b1, 20) < 0 || virBitmapParse("4,9,10,16-19", 0, &b2, 20) < 0 || - virBitmapParse("15", 0, &b3, 20) < 0) + virBitmapParse("15", 0, &b3, 20) < 0 || + virBitmapParse("0,^0", 0, &b4, 20) < 0) + goto cleanup; + + if (!virBitmapIsAllClear(b4)) goto cleanup; if (virBitmapOverlaps(b1, b2) || + virBitmapOverlaps(b1, b4) || virBitmapOverlaps(b2, b3) || - !virBitmapOverlaps(b1, b3)) + virBitmapOverlaps(b2, b4) || + !virBitmapOverlaps(b1, b3) || + virBitmapOverlaps(b3, b4)) goto cleanup; ret = 0; -- 1.9.3

3 4

[libvirt] qemu: lifecycle: reboot + shutdown, unexpected vm status.
by zhang bo 16 Apr '15

16 Apr '15

Steps: 1 virsh reboot guest1 --mode=acpi 2 virsh shutdown guest1 --mode=agent Expected result: As the SHUTDOWN job is after REBOOT, we expected the guest to be *shutoff*. (Do you think so?) Exacted result: After the 2 steps above, the guest got *rebooted*. The reason to this problem: 1 in qemuDomainReboot(mode acpi), it sets priv->fakeReboot to 1. 2 after shutdown/reboot, qemu monitor IO trigged qemuProcessHandleShutdown(), which finds that priv->fakeReboot is 1, and reboot the guest. Root Cause of the problem: After further look into the problem, We found that the design of acpi/agent shutdown/reboot seems a little chaotic. ----------------------------------- sheet1 who sets fakeReboot ----------------------------------- shutdown reboot acpi Y(0) Y(1) agent N N It's apparently, *acpi-mode* jobs set fakeReboot. ----------------------------------- sheet2 who needs to check fakeReboot(qemuProcessHandleShutdown()) ----------------------------------- shutdown reboot acpi Y Y agent *Y* *N* Things become a little odd here. only agent-mode reboot doesn't check fakeReboot. We can tell from the above 2 sheets, that they're not consistent. *Agent-mode shutdown needs to check fakeReboot(trigger by SHUTDOWN monitorIO event), while it didn't set it before.* The chaos is not caused by libvirtd, it's a systematic problem, including guest os and qemu. (guest os writes ACPI, triggers qemu, qemu then triggers libvirtd) My Solution: A simple solution is to make the 1st sheet consistent to the 2nd sheet, which is: ----------------------------------- sheet3 who should set fakeReboot: ----------------------------------- shutdown reboot acpi Y(0) Y(1) agent *Y(0)* N ----------------------------------- we let agent-mode shutdown set fakeReboot to 0. -------------------------------------------------------------------------------- diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7eb5a7d..c751dcf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1959,6 +1959,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) goto endjob; } + qemuDomainSetFakeReboot(driver, vm, isReboot); + if (useAgent) { qemuDomainObjEnterAgent(vm); ret = qemuAgentShutdown(priv->agent, agentFlag); @@ -1970,7 +1972,6 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) */ if (!useAgent || (ret < 0 && (acpiRequested || !flags))) { - qemuDomainSetFakeReboot(driver, vm, isReboot); /* Even if agent failed, we have to check if guest went away * by itself while our locks were down. */ ---------------------------------------------------------------------------------- Discussion: Although the solution above seems to have solved the problem, but it seems a little awkward to have sheets like this. Maybe we should let sheet2 consistent with sheet1, rather than letting sheet1 consistent to sheet2. But It's too difficult to realize that, seems impossible So, is there a better way to solve this problem? or, shall I commit this patch?

2 1

Re: [libvirt] Building libvirt under Windows
by Matthias Bolte 16 Apr '15

16 Apr '15

2015-04-16 11:52 GMT+02:00 Pavel Fedin <p.fedin(a)samsung.com>: > Hello! > > I am currently building libvirt under Windows64, and i would like to ask > for correct way to do it. > The first thing to resolve is absence of RPC XDR library. From configure > source i figured out that portablexdr library may help. I have downloaded > it, but it comes with a different RPC code generator named portable-rpcgen. > I managed to use it, but i had to patch both libvirt (in order to recognize > this generator and handle it properly) and portable-rpcgen (in order to > correctly work under Windows, as well as get rid of some crashes). > Initially i wanted to upstream portablexdr fixes and emailed the author. > But the author replied that he is not going to fix portablexdr because it's > now deprecated, since original XDR code comes under free license now. So, i > have all necessary fixes in my own fork. He suggested me to come here to ask > about these things. > So, guys, how do you do it ? I cannot find any rpcgen package usable under > Windows (MinGW, not Cygwin). Hi, In 2010 I wrote a set of scripts to build libvirt on Windows using MinGW. Sou can find them here: https://github.com/photron/msys_setup I haven't update them in the last two years, so they might not work out-of-the-box anymore. But it might still be helpful to look at them and see how I did things. Regarding XDR: I used portablexdr. It worked for me, I can't remember if I had to patch it or not. -- Matthias Bolte http://photron.blogspot.com

1 0

[libvirt] util: client: shall libvirtd record task histories?
by zhang bo 16 Apr '15

16 Apr '15

We recently encountered a problem: 1) migrate a domain 2) the client unexpectedly got *crashed* (let's take it as virsh command) 3) *libvirtd still kept migrating the domain* 4) after it's restarted, the client didn't know the guest is still migrating. The problem is that libvirtd and the client has different view of the task state. After migration, the client may wrongly think that something's wrong that the domain got unexpectedly migrated. In my opinion, libvirtd should just *execute* tasks, like the hands of a human, while clients should be the brain to *schedule and remember* tasks. So, In order to avoid this problem,we should let the client record all the taskes somewhere, and reload the states after its restart. the client may cancel or continue the task as it wishes. Libvirtd should not record the task status. What's your opinions? thanks in advance.

3 3

[libvirt] [PATCH] qemu: bulk stats: Ignore errors from missing/inaccessible disks
by Peter Krempa 16 Apr '15

16 Apr '15

Rather than erroring out make the best attempt to retrieve other data if disks are inaccessible or missing. The failure will still be logged though. Since the bulk stats API is called on multiple domains an error like this makes the API unusable. This regression was introduced by commit 596a13713420e01b20ce3dc3fdbe06d073682675 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1209394 --- src/qemu/qemu_driver.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f5a3ef9..e79e2a9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19275,8 +19275,13 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, ret = 0; goto cleanup; } - if (qemuStorageLimitsRefresh(driver, cfg, dom, src) < 0) + + if (qemuStorageLimitsRefresh(driver, cfg, dom, src) < 0) { + virResetLastError(); + ret = 0; goto cleanup; + } + if (src->allocation) QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, "allocation", src->allocation); -- 2.3.5

2 2

[libvirt] [java PATCH] added pom details
by Laszlo Hornyak 15 Apr '15

15 Apr '15

The added details are required in order to upload to maven central Signed-off-by: Laszlo Hornyak <laszlo.hornyak(a)gmail.com> --- pom.xml.in | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pom.xml.in b/pom.xml.in index 4f49a3a..2ac8822 100644 --- a/pom.xml.in +++ b/pom.xml.in @@ -12,6 +12,18 @@ <name>libvirt java bindings</name> <description>Java API for the libvirt C library</description> <url>http://www.libvirt.org</url> + <developers> + <developer> + <id>veillard(a)redhat.com</id> + <email>veillard(a)redhat.com</email> + <organization>redhat</organization> + <organizationUrl>http://www.redhat.com</organizationUrl> + </developer> + </developers> + <organization> + <name>Libvirt</name> + <url>https://libvirt.org/</url> + </organization> <licenses> <license> <name>MIT license</name> @@ -20,6 +32,8 @@ </licenses> <scm> <url>http://www.libvirt.org/git/?p=libvirt-java.git;a=summary</url> + <developerConnection>scm:git://libvirt.org/libvirt-java.git</developerConnection> + <connection>scm:git://libvirt.org/libvirt-java.git</connection> </scm> <dependencies> -- 1.9.3

1 0

[libvirt] [PATCH] added pom details
by Laszlo Hornyak 15 Apr '15

15 Apr '15

The added details are required in order to upload to maven central Signed-off-by: Laszlo Hornyak <laszlo.hornyak(a)gmail.com> --- pom.xml.in | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pom.xml.in b/pom.xml.in index 4f49a3a..2ac8822 100644 --- a/pom.xml.in +++ b/pom.xml.in @@ -12,6 +12,18 @@ <name>libvirt java bindings</name> <description>Java API for the libvirt C library</description> <url>http://www.libvirt.org</url> + <developers> + <developer> + <id>veillard(a)redhat.com</id> + <email>veillard(a)redhat.com</email> + <organization>redhat</organization> + <organizationUrl>http://www.redhat.com</organizationUrl> + </developer> + </developers> + <organization> + <name>Libvirt</name> + <url>https://libvirt.org/</url> + </organization> <licenses> <license> <name>MIT license</name> @@ -20,6 +32,8 @@ </licenses> <scm> <url>http://www.libvirt.org/git/?p=libvirt-java.git;a=summary</url> + <developerConnection>scm:git://libvirt.org/libvirt-java.git</developerConnection> + <connection>scm:git://libvirt.org/libvirt-java.git</connection> </scm> <dependencies> -- 1.9.3

2 2

[libvirt] [PATCH] Fix virCgroupGetPercpuStats with non-continuous present CPUs
by Ján Tomko 15 Apr '15

15 Apr '15

Per-cpu stats are only shown for present CPUs in the cgroups, but we were only parsing the largest CPU number from /sys/devices/system/cpu/present and looking for stats even for non-present CPUs. This resulted in: internal error: cpuacct parse error --- cfg.mk | 2 +- src/nodeinfo.c | 17 +++++++++++++++++ src/nodeinfo.h | 1 + src/util/vircgroup.c | 24 ++++++++++++++++++------ tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------ tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------ 6 files changed, 110 insertions(+), 25 deletions(-) diff --git a/cfg.mk b/cfg.mk index 21f83c3..70612f8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1095,7 +1095,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$) exclude_file_name_regexp--sc_prohibit_strdup = \ - ^(docs/|examples/|src/util/virstring\.c|tests/virnetserverclientmock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$) exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 3c22ebc..3a27c22 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1242,6 +1242,23 @@ nodeGetCPUCount(void) } virBitmapPtr +nodeGetPresentCPUBitmap(void) +{ + int max_present; + + if ((max_present = nodeGetCPUCount()) < 0) + return NULL; + +#ifdef __linux__ + if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) + return linuxParseCPUmap(max_present, SYSFS_SYSTEM_PATH "/cpu/present"); +#endif + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("non-continuous host cpu numbers not implemented on this platform")); + return NULL; +} + +virBitmapPtr nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED) { #ifdef __linux__ diff --git a/src/nodeinfo.h b/src/nodeinfo.h index a993c63..047bd5c 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -43,6 +43,7 @@ int nodeGetCellsFreeMemory(unsigned long long *freeMems, int nodeGetMemory(unsigned long long *mem, unsigned long long *freeMem); +virBitmapPtr nodeGetPresentCPUBitmap(void); virBitmapPtr nodeGetCPUBitmap(int *max_id); int nodeGetCPUCount(void); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index fe34290..e65617a 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2985,7 +2985,8 @@ static int virCgroupGetPercpuVcpuSum(virCgroupPtr group, unsigned int nvcpupids, unsigned long long *sum_cpu_time, - unsigned int num) + size_t nsum, + virBitmapPtr cpumap) { int ret = -1; size_t i; @@ -2995,7 +2996,7 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, for (i = 0; i < nvcpupids; i++) { char *pos; unsigned long long tmp; - size_t j; + ssize_t j; if (virCgroupNewVcpu(group, i, false, &group_vcpu) < 0) goto cleanup; @@ -3004,7 +3005,9 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, goto cleanup; pos = buf; - for (j = 0; j < num; j++) { + for (j = virBitmapNextSetBit(cpumap, -1); + j >= 0 && j < nsum; + j = virBitmapNextSetBit(cpumap, j)) { if (virStrToLong_ull(pos, &pos, 10, &tmp) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); @@ -3042,6 +3045,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; + virBitmapPtr cpumap = NULL; /* return the number of supported params */ if (nparams == 0 && ncpus != 0) { @@ -3052,9 +3056,11 @@ virCgroupGetPercpuStats(virCgroupPtr group, } /* To parse account file, we need to know how many cpus are present. */ - if ((total_cpus = nodeGetCPUCount()) < 0) + if (!(cpumap = nodeGetPresentCPUBitmap())) return rv; + total_cpus = virBitmapSize(cpumap); + if (ncpus == 0) return total_cpus; @@ -3077,7 +3083,11 @@ virCgroupGetPercpuStats(virCgroupPtr group, need_cpus = MIN(total_cpus, start_cpu + ncpus); for (i = 0; i < need_cpus; i++) { - if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { + bool present; + ignore_value(virBitmapGetBit(cpumap, i, &present)); + if (!present) { + cpu_time = 0; + } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); goto cleanup; @@ -3097,7 +3107,8 @@ virCgroupGetPercpuStats(virCgroupPtr group, if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0) goto cleanup; - if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus) < 0) + if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus, + cpumap) < 0) goto cleanup; for (i = start_cpu; i < need_cpus; i++) { @@ -3113,6 +3124,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, rv = param_idx + 1; cleanup: + virBitmapFree(cpumap); VIR_FREE(sum_cpu_time); VIR_FREE(buf); return rv; diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 6e0a0e7..f2fee7d 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -51,6 +51,8 @@ const char *fakedevicedir1 = FAKEDEVDIR1; # define SYSFS_PREFIX "/not/really/sys/fs/cgroup/" +# define SYSFS_CPU_PRESENT "/sys/devices/system/cpu/present" +# define SYSFS_CPU_PRESENT_MOCKED "devices_system_cpu_present" /* * The plan: @@ -215,7 +217,15 @@ static int make_controller(const char *path, mode_t mode) "user 216687025\n" "system 43421396\n"); MAKE_FILE("cpuacct.usage", "2787788855799582\n"); - MAKE_FILE("cpuacct.usage_percpu", "1413142688153030 1374646168910542\n"); + MAKE_FILE("cpuacct.usage_percpu", + "7059492996 0 0 0 0 0 0 0 4180532496 0 0 0 0 0 0 0 " + "1957541268 0 0 0 0 0 0 0 2065932204 0 0 0 0 0 0 0 " + "18228689414 0 0 0 0 0 0 0 4245525148 0 0 0 0 0 0 0 " + "2911161568 0 0 0 0 0 0 0 1407758136 0 0 0 0 0 0 0 " + "1836807700 0 0 0 0 0 0 0 1065296618 0 0 0 0 0 0 0 " + "2046213266 0 0 0 0 0 0 0 747889778 0 0 0 0 0 0 0 " + "709566900 0 0 0 0 0 0 0 444777342 0 0 0 0 0 0 0 " + "5683512916 0 0 0 0 0 0 0 635751356 0 0 0 0 0 0 0\n"); } else if (STRPREFIX(controller, "cpuset")) { MAKE_FILE("cpuset.cpu_exclusive", "1\n"); if (STREQ(controller, "cpuset")) @@ -431,6 +441,9 @@ static void init_sysfs(void) MAKE_CONTROLLER("blkio"); MAKE_CONTROLLER("memory"); MAKE_CONTROLLER("freezer"); + + if (make_file(fakesysfsdir, SYSFS_CPU_PRESENT_MOCKED, "8-23,48-159\n") < 0) + abort(); } @@ -623,21 +636,27 @@ int __xstat(int ver, const char *path, struct stat *sb) int stat(const char *path, struct stat *sb) { + char *newpath = NULL; int ret; init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STREQ(path, SYSFS_CPU_PRESENT)) { + init_sysfs(); + if (asprintf(&newpath, "%s/%s", + fakesysfsdir, + SYSFS_CPU_PRESENT_MOCKED) < 0) { + errno = ENOMEM; + return -1; + } + } else if (STRPREFIX(path, SYSFS_PREFIX)) { init_sysfs(); - char *newpath; if (asprintf(&newpath, "%s/%s", fakesysfsdir, path + strlen(SYSFS_PREFIX)) < 0) { errno = ENOMEM; return -1; } - ret = realstat(newpath, sb); - free(newpath); } else if (STRPREFIX(path, fakedevicedir0)) { sb->st_mode = S_IFBLK; sb->st_rdev = makedev(8, 0); @@ -647,8 +666,11 @@ int stat(const char *path, struct stat *sb) sb->st_rdev = makedev(9, 0); return 0; } else { - ret = realstat(path, sb); + if (!(newpath = strdup(path))) + return -1; } + ret = realstat(newpath, sb); + free(newpath); return ret; } @@ -682,6 +704,16 @@ int open(const char *path, int flags, ...) init_syms(); + if (STREQ(path, SYSFS_CPU_PRESENT)) { + init_sysfs(); + if (asprintf(&newpath, "%s/%s", + fakesysfsdir, + SYSFS_CPU_PRESENT_MOCKED) < 0) { + errno = ENOMEM; + return -1; + } + } + if (STRPREFIX(path, SYSFS_PREFIX)) { init_sysfs(); if (asprintf(&newpath, "%s/%s", diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 35ac0c0..b65ea3f 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -538,12 +538,35 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) virCgroupPtr cgroup = NULL; size_t i; int rv, ret = -1; - virTypedParameter params[2]; + virTypedParameterPtr params = NULL; +# define EXPECTED_NCPUS 160 - // TODO: mock nodeGetCPUCount() as well & check 2nd cpu, too unsigned long long expected[] = { - 1413142688153030ULL + 0, 0, 0, 0, 0, 0, 0, 0, + 7059492996, 0, 0, 0, 0, 0, 0, 0, + 4180532496, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 1957541268, 0, 0, 0, 0, 0, 0, 0, + 2065932204, 0, 0, 0, 0, 0, 0, 0, + 18228689414, 0, 0, 0, 0, 0, 0, 0, + 4245525148, 0, 0, 0, 0, 0, 0, 0, + 2911161568, 0, 0, 0, 0, 0, 0, 0, + 1407758136, 0, 0, 0, 0, 0, 0, 0, + 1836807700, 0, 0, 0, 0, 0, 0, 0, + 1065296618, 0, 0, 0, 0, 0, 0, 0, + 2046213266, 0, 0, 0, 0, 0, 0, 0, + 747889778, 0, 0, 0, 0, 0, 0, 0, + 709566900, 0, 0, 0, 0, 0, 0, 0, + 444777342, 0, 0, 0, 0, 0, 0, 0, + 5683512916, 0, 0, 0, 0, 0, 0, 0, + 635751356, 0, 0, 0, 0, 0, 0, 0, }; + verify(ARRAY_CARDINALITY(expected) == EXPECTED_NCPUS); + + if (VIR_ALLOC_N(params, EXPECTED_NCPUS) < 0) + goto cleanup; if ((rv = virCgroupNewPartition("/virtualmachines", true, (1 << VIR_CGROUP_CONTROLLER_CPU) | @@ -553,37 +576,37 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - if (nodeGetCPUCount() < 1) { + if (nodeGetCPUCount() != EXPECTED_NCPUS) { fprintf(stderr, "Unexpected: nodeGetCPUCount() yields: %d\n", nodeGetCPUCount()); goto cleanup; } if ((rv = virCgroupGetPercpuStats(cgroup, params, - 2, 0, 1, 0)) < 0) { + 1, 0, EXPECTED_NCPUS, 0)) < 0) { fprintf(stderr, "Failed call to virCgroupGetPercpuStats for /virtualmachines cgroup: %d\n", -rv); goto cleanup; } - for (i = 0; i < ARRAY_CARDINALITY(expected); i++) { + for (i = 0; i < EXPECTED_NCPUS; i++) { if (!STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) { fprintf(stderr, - "Wrong parameter name value from virCgroupGetPercpuStats (is: %s)\n", - params[i].field); + "Wrong parameter name value from virCgroupGetPercpuStats at %zu (is: %s)\n", + i, params[i].field); goto cleanup; } if (params[i].type != VIR_TYPED_PARAM_ULLONG) { fprintf(stderr, - "Wrong parameter value type from virCgroupGetPercpuStats (is: %d)\n", - params[i].type); + "Wrong parameter value type from virCgroupGetPercpuStats at %zu (is: %d)\n", + i, params[i].type); goto cleanup; } if (params[i].value.ul != expected[i]) { fprintf(stderr, - "Wrong value from virCgroupGetMemoryUsage (expected %llu)\n", - params[i].value.ul); + "Wrong value from virCgroupGetMemoryUsage at %zu (expected %llu)\n", + i, params[i].value.ul); goto cleanup; } } -- 2.0.4

3 4

[libvirt] [PATCH] tests: fix build on old 32-bit platforms
by Eric Blake 15 Apr '15

15 Apr '15

gcc 4.1.2 (hello RHEL 5) on 32-bit platforms complains: vircgrouptest.c: In function 'testCgroupGetPercpuStats': vircgrouptest.c:627: warning: integer constant is too large for 'long' type vircgrouptest.c:628: warning: this decimal constant is unsigned only in ISO C90 vircgrouptest.c:634: warning: integer constant is too large for 'long' type vircgrouptest.c:635: warning: this decimal constant is unsigned only in ISO C90 vircgrouptest.c:636: warning: this decimal constant is unsigned only in ISO C90 vircgrouptest.c:644: warning: integer constant is too large for 'long' type * tests/vircgrouptest.c (testCgroupGetPercpuStats): Use ULL suffix. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule tests/vircgrouptest.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 2b30258..ad49425 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013-2014 Red Hat, Inc. + * Copyright (C) 2013-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -624,25 +624,25 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) unsigned long long expected[EXPECTED_NCPUS] = { 0, 0, 0, 0, 0, 0, 0, 0, - 7059492996, 0, 0, 0, 0, 0, 0, 0, - 4180532496, 0, 0, 0, 0, 0, 0, 0, + 7059492996ULL, 0, 0, 0, 0, 0, 0, 0, + 4180532496ULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 1957541268, 0, 0, 0, 0, 0, 0, 0, - 2065932204, 0, 0, 0, 0, 0, 0, 0, - 18228689414, 0, 0, 0, 0, 0, 0, 0, - 4245525148, 0, 0, 0, 0, 0, 0, 0, - 2911161568, 0, 0, 0, 0, 0, 0, 0, - 1407758136, 0, 0, 0, 0, 0, 0, 0, - 1836807700, 0, 0, 0, 0, 0, 0, 0, - 1065296618, 0, 0, 0, 0, 0, 0, 0, - 2046213266, 0, 0, 0, 0, 0, 0, 0, - 747889778, 0, 0, 0, 0, 0, 0, 0, - 709566900, 0, 0, 0, 0, 0, 0, 0, - 444777342, 0, 0, 0, 0, 0, 0, 0, - 5683512916, 0, 0, 0, 0, 0, 0, 0, - 635751356, 0, 0, 0, 0, 0, 0, 0, + 1957541268ULL, 0, 0, 0, 0, 0, 0, 0, + 2065932204ULL, 0, 0, 0, 0, 0, 0, 0, + 18228689414ULL, 0, 0, 0, 0, 0, 0, 0, + 4245525148ULL, 0, 0, 0, 0, 0, 0, 0, + 2911161568ULL, 0, 0, 0, 0, 0, 0, 0, + 1407758136ULL, 0, 0, 0, 0, 0, 0, 0, + 1836807700ULL, 0, 0, 0, 0, 0, 0, 0, + 1065296618ULL, 0, 0, 0, 0, 0, 0, 0, + 2046213266ULL, 0, 0, 0, 0, 0, 0, 0, + 747889778ULL, 0, 0, 0, 0, 0, 0, 0, + 709566900ULL, 0, 0, 0, 0, 0, 0, 0, + 444777342ULL, 0, 0, 0, 0, 0, 0, 0, + 5683512916ULL, 0, 0, 0, 0, 0, 0, 0, + 635751356ULL, 0, 0, 0, 0, 0, 0, 0, }; if (VIR_ALLOC_N(params, EXPECTED_NCPUS) < 0) -- 2.1.0

1 0

[libvirt] [PATCH] virNetSocketNewConnectUNIX: Use flocks when spawning a daemon
by Michal Privoznik 15 Apr '15

15 Apr '15

https://bugzilla.redhat.com/show_bug.cgi?id=1200149 Even though we have a mutex mechanism so that two clients don't spawn two daemons, it's not strong enough. It can happen that while one client is spawning the daemon, the other one fails to connect. Basically two possible errors can happen: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused or: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory The problem in both cases is, the daemon is only starting up, while we are trying to connect (and fail). We should postpone the connecting phase until the daemon is started (by the other thread that is spawning it). In order to do that, create a file lock 'libvirt-lock' in the directory where session daemon would create its socket. So even when called from multiple processes, spawning a daemon will serialize on the file lock. So only the first to come will spawn the daemon. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- diff to v1: -hopefully this one is race free src/rpc/virnetsocket.c | 148 ++++++++++++------------------------------------- 1 file changed, 36 insertions(+), 112 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 6b019cc..2b891ae 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -123,7 +123,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket) #ifndef WIN32 -static int virNetSocketForkDaemon(const char *binary, int passfd) +static int virNetSocketForkDaemon(const char *binary) { int ret; virCommandPtr cmd = virCommandNewArgList(binary, @@ -136,10 +136,6 @@ static int virNetSocketForkDaemon(const char *binary, int passfd) virCommandAddEnvPassBlockSUID(cmd, "XDG_RUNTIME_DIR", NULL); virCommandClearCaps(cmd); virCommandDaemonize(cmd); - if (passfd) { - virCommandPassFD(cmd, passfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); - virCommandPassListenFDs(cmd); - } ret = virCommandRun(cmd, NULL); virCommandFree(cmd); return ret; @@ -543,10 +539,10 @@ int virNetSocketNewConnectUNIX(const char *path, const char *binary, virNetSocketPtr *retsock) { - char *binname = NULL; - char *pidpath = NULL; - int fd, passfd = -1; - int pidfd = -1; + char *lockpath = NULL; + int lockfd = -1; + int fd = -1; + int retries = 100; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -561,6 +557,22 @@ int virNetSocketNewConnectUNIX(const char *path, return -1; } + if (spawnDaemon) { + if (virPidFileConstructPath(false, NULL, "libvirt-lock", &lockpath) < 0) + goto error; + + if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0600)) < 0 || + virSetCloseExec(lockfd) < 0) { + virReportSystemError(errno, _("Unable to create lock '%s'"), lockpath); + goto error; + } + + if (virFileLock(lockfd, false, 0, 1, true) < 0) { + virReportSystemError(errno, _("Unable to lock '%s'"), lockpath); + goto error; + } + } + if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { virReportSystemError(errno, "%s", _("Failed to create socket")); goto error; @@ -574,108 +586,25 @@ int virNetSocketNewConnectUNIX(const char *path, if (remoteAddr.data.un.sun_path[0] == '@') remoteAddr.data.un.sun_path[0] = '\0'; - retry: - if (connect(fd, &remoteAddr.data.sa, remoteAddr.len) < 0) { - int status = 0; - pid_t pid = 0; - - if (!spawnDaemon) { + while (retries && + connect(fd, &remoteAddr.data.sa, remoteAddr.len) < 0) { + if (!(spawnDaemon && errno == ENOENT)) { virReportSystemError(errno, _("Failed to connect socket to '%s'"), path); goto error; } - if (!(binname = last_component(binary)) || binname[0] == '\0') { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot determine basename for binary '%s'"), - binary); - goto error; - } - - if (virPidFileConstructPath(false, NULL, binname, &pidpath) < 0) - goto error; - - pidfd = virPidFileAcquirePath(pidpath, false, getpid()); - if (pidfd < 0) { - /* - * This can happen in a very rare case of two clients spawning two - * daemons at the same time, and the error in the logs that gets - * reset here can be a clue to some future debugging. - */ - virResetLastError(); - spawnDaemon = false; - goto retry; - } - - if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { - virReportSystemError(errno, "%s", _("Failed to create socket")); + if (virNetSocketForkDaemon(binary) < 0) goto error; - } - /* - * We already even acquired the pidfile, so no one else should be using - * @path right now. So we're OK to unlink it and paying attention to - * the return value makes no real sense here. Only if it's not an - * abstract socket, of course. - */ - if (path[0] != '@') - unlink(path); - - /* - * We have to fork() here, because umask() is set per-process, chmod() - * is racy and fchmod() has undefined behaviour on sockets according to - * POSIX, so it doesn't work outside Linux. - */ - if ((pid = virFork()) < 0) - goto error; - - if (pid == 0) { - umask(0077); - if (bind(passfd, &remoteAddr.data.sa, remoteAddr.len) < 0) - _exit(EXIT_FAILURE); - - _exit(EXIT_SUCCESS); - } + retries--; + usleep(5000); + } - if (virProcessWait(pid, &status, false) < 0) - goto error; - - if (status != EXIT_SUCCESS) { - /* - * OK, so the child failed to bind() the socket. This may mean that - * another daemon was starting at the same time and succeeded with - * its bind() (even though it should not happen because we using a - * pidfile for the race). So we'll try connecting again, but this - * time without spawning the daemon. - */ - spawnDaemon = false; - virPidFileDeletePath(pidpath); - VIR_FORCE_CLOSE(pidfd); - VIR_FORCE_CLOSE(passfd); - goto retry; - } - - if (listen(passfd, 0) < 0) { - virReportSystemError(errno, "%s", - _("Failed to listen on socket that's about " - "to be passed to the daemon")); - goto error; - } - - if (connect(fd, &remoteAddr.data.sa, remoteAddr.len) < 0) { - virReportSystemError(errno, _("Failed to connect socket to '%s'"), - path); - goto error; - } - - /* - * Do we need to eliminate the super-rare race here any more? It would - * need incorporating the following VIR_FORCE_CLOSE() into a - * virCommandHook inside a virNetSocketForkDaemon(). - */ - VIR_FORCE_CLOSE(pidfd); - if (virNetSocketForkDaemon(binary, passfd) < 0) - goto error; + if (lockfd) { + unlink(lockpath); + VIR_FORCE_CLOSE(lockfd); + VIR_FREE(lockpath); } localAddr.len = sizeof(localAddr.data); @@ -687,19 +616,14 @@ int virNetSocketNewConnectUNIX(const char *path, if (!(*retsock = virNetSocketNew(&localAddr, &remoteAddr, true, fd, -1, 0))) goto error; - VIR_FREE(pidpath); - return 0; error: - if (pidfd >= 0) - virPidFileDeletePath(pidpath); - VIR_FREE(pidpath); + if (lockfd) + unlink(lockpath); + VIR_FREE(lockpath); VIR_FORCE_CLOSE(fd); - VIR_FORCE_CLOSE(passfd); - VIR_FORCE_CLOSE(pidfd); - if (spawnDaemon) - unlink(path); + VIR_FORCE_CLOSE(lockfd); return -1; } #else -- 2.0.5

4 3

[libvirt] [PATCHv2 0/5] Strip control codes from virBufferEscapeString
by Ján Tomko 15 Apr '15

15 Apr '15

For https://bugzilla.redhat.com/show_bug.cgi?id=1184131 https://bugzilla.redhat.com/show_bug.cgi?id=1066564 v1: https://www.redhat.com/archives/libvir-list/2015-March/msg01529.html v2: * added patches 1-4 that should prevent us from calling virBufferEscapeString with control codes * patch 5 is unchanged Ján Tomko (5): tests: rename testStripIPv6BracketsData to testStripData Add functions dealing with control characters in strings Strip control characters from sysfs attributes Ignore storage volumes with control codes in their names Strip control codes in virBufferEscapeString src/libvirt_private.syms | 2 ++ src/node_device/node_device_udev.c | 2 ++ src/storage/storage_backend_fs.c | 6 +++++ src/util/virbuffer.c | 14 ++++++++--- src/util/virstring.c | 39 ++++++++++++++++++++++++++++++ src/util/virstring.h | 2 ++ tests/virbuftest.c | 49 ++++++++++++++++++++++++++++++++++++++ tests/virstringtest.c | 45 +++++++++++++++++++++++++++++++--- 8 files changed, 153 insertions(+), 6 deletions(-) -- 2.0.5

2 11

[libvirt] [PATCH 0/2] Couple of virnetdevtest cleanups
by Michal Privoznik 15 Apr '15

15 Apr '15

*** BLURB HERE *** Michal Privoznik (2): tests: Add virnetdevtestdata to EXTRA_DIST Cleanup "/sys/class/net" usage src/Makefile.am | 4 ++-- src/parallels/parallels_network.c | 3 +-- src/util/virnetdev.c | 5 ++--- src/util/virnetdev.h | 2 ++ src/util/virnetdevbridge.c | 1 - src/util/virnetdevmacvlan.c | 28 ++++++++++++++-------------- tests/Makefile.am | 1 + 7 files changed, 22 insertions(+), 22 deletions(-) -- 2.0.5

2 5

[libvirt] [PATCH v2] Link libvirt_util with datatypes
by Martin Kletzander 15 Apr '15

15 Apr '15

Most of the types in datatypes.[hc] depend on virObject and virClass, but they were specified separatedly from that. We were lucky enough for this to work because wherever the datatypes files were used, that file (binary/shared object) was linked to libvirt_util as well. Fixing this comes up as a pretty nice cleanup. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/Makefile.am | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 91a4c17..84f489f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-2015 Red Hat, Inc. ## ## This library is free software; you can redistribute it and/or ## modify it under the terms of the GNU Lesser General Public @@ -86,9 +86,12 @@ augeas_DATA = augeastestdir = $(datadir)/augeas/lenses/tests augeastest_DATA = +DATATYPES_SOURCES = datatypes.h datatypes.c + # These files are not related to driver APIs. Simply generic # helper APIs for various purposes UTIL_SOURCES = \ + $(DATATYPES_SOURCES) \ util/viralloc.c util/viralloc.h \ util/virarch.h util/virarch.c \ util/viratomic.h util/viratomic.c \ @@ -185,7 +188,6 @@ util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ # Internal generic driver infrastructure NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c nodeinfopriv.h -DATATYPES_SOURCES = datatypes.h datatypes.c DRIVER_SOURCES = \ driver.c driver.h \ driver-hypervisor.h \ @@ -198,7 +200,6 @@ DRIVER_SOURCES = \ driver-storage.h \ driver-stream.h \ internal.h \ - $(DATATYPES_SOURCES) \ fdstream.c fdstream.h \ $(NODE_INFO_SOURCES) \ libvirt.c libvirt_internal.h \ @@ -671,10 +672,6 @@ LXC_CONTROLLER_SOURCES = \ lxc/lxc_fuse.c lxc/lxc_fuse.h \ lxc/lxc_controller.c -SECURITY_DRIVER_APPARMOR_HELPER_SOURCES = \ - $(DATATYPES_SOURCES) \ - security/virt-aa-helper.c - PHYP_DRIVER_SOURCES = \ phyp/phyp_driver.c phyp/phyp_driver.h @@ -2604,8 +2601,7 @@ libexec_PROGRAMS += libvirt_lxc libvirt_lxc_SOURCES = \ $(LXC_CONTROLLER_SOURCES) \ - $(NODE_INFO_SOURCES) \ - $(DATATYPES_SOURCES) + $(NODE_INFO_SOURCES) libvirt_lxc_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ @@ -2643,7 +2639,7 @@ if WITH_SECDRIVER_APPARMOR if WITH_LIBVIRTD libexec_PROGRAMS += virt-aa-helper -virt_aa_helper_SOURCES = $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) +virt_aa_helper_SOURCES = security/virt-aa-helper.c virt_aa_helper_LDFLAGS = \ $(AM_LDFLAGS) \ @@ -2666,7 +2662,7 @@ virt_aa_helper_CFLAGS = \ $(NULL) endif WITH_LIBVIRTD endif WITH_SECDRIVER_APPARMOR -EXTRA_DIST += $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) +EXTRA_DIST += security/virt-aa-helper.c install-data-local: install-init install-systemd if WITH_LIBVIRTD -- 2.3.5

3 4