[libvirt] [test-API][PATCH] Add connection_security_model test case
The connection_security_model.py uses getSecurityModel() to validate new API virNodeGetSecurityModel of libvirt. --- cases/linux_domain.conf| 4 ++ repos/virconn/connection_security_model.py | 101 + 2 files changed, 105 insertions(+) create mode 100644 repos/virconn/connection_security_model.py diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf index 903fdb5..a7015f0 100644 --- a/cases/linux_domain.conf +++ b/cases/linux_domain.conf @@ -233,6 +233,10 @@ domain:domain_fsthaw guestname $defaultname +virconn:connection_security_model +guestname +$defaultname + domain:destroy guestname $defaultname diff --git a/repos/virconn/connection_security_model.py b/repos/virconn/connection_security_model.py new file mode 100644 index 000..b44d78c --- /dev/null +++ b/repos/virconn/connection_security_model.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python +# To test "getSecurityModel" + +import libvirt + +from xml.dom import minidom +from libvirt import libvirtError +from src import sharedmod +from utils import utils + +required_params = ('guestname',) +optional_params = {} + +def get_security_driver(logger): +"""get security driver from /etc/libvirt/qemu.conf""" + +cmds = "grep \"^security_driver\" /etc/libvirt/qemu.conf" +(ret, conf) = utils.exec_cmd(cmds, shell=True) +if ret: +cmds = "getenforce" +(ret, policy) = utils.exec_cmd(cmds, shell=True) + +if policy[0] == "Disabled": +return "none" +else: +return "selinux" + +tmp = conf[0].split(' = ') +if len(tmp[1].split(', ')) > 1: +driver = tmp[1].split(', ') +return (filter(str.isalpha, driver[0])) +else: +cmds = "echo '%s' | awk -F '\"' '{print $2}'" % conf[0] +(ret, driver) = utils.exec_cmd(cmds, shell=True) + +if driver[0] == "selinux": +return "selinux" +elif driver[0] == "none": +return "none" +elif driver[0] == "apparmor": +return "apparmor" +elif driver[0] == "stack": +return "stack" +else: +return "" + +def get_security_model(logger, domname): +"""get security model from process""" + +PID = "ps aux | grep -v grep | grep %s | awk '{print $2}'" % domname +ret, pid = utils.exec_cmd(PID, shell=True) +if ret: +logger.error("get domain pid failed.") +return "" + +LABEL = "ls -nZd /proc/%s" % pid[0] +ret, label = utils.exec_cmd(LABEL, shell=True) +if ret: +logger.error("get domain process's label failed.") +return "" + +if "system_u:system_r:svirt_t:s0" in label[0]: +return "selinux" +else: +return "none" + +def check_security_model(logger, domname, model): +""" check security model""" + +dommodel = get_security_model(logger, domname) +driver = get_security_driver(logger) + +logger.info("domain security model is %s." % dommodel) +logger.info("get security driver is %s." % driver) +logger.info("get security model is %s." % model) + +if driver == dommodel and dommodel == model: +return True +else: +return False + +def connection_security_model(params): +"""test API for getSecurityModel""" + +logger = params['logger'] +domname = params['guestname'] +conn = sharedmod.libvirtobj['conn'] + +try: +model = conn.getSecurityModel() + +if not check_security_model(logger, domname, model[0]): +logger.error("Fail : get a error security model.") +return 1 +else: +logger.info("Pass : get security model successful.") +return 0 +except libvirtError, e: +logger.error("API error message: %s" % e.message) +return 1 + -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix some wrong indentation or types
--- src/qemu/qemu_command.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa7a928..02105c3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1683,8 +1683,8 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, * */ if (nbuses > 0) - virDomainPCIAddressBusSetModel(&addrs->buses[0], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); +virDomainPCIAddressBusSetModel(&addrs->buses[0], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); for (i = 1; i < nbuses; i++) { virDomainPCIAddressBusSetModel(&addrs->buses[i], VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); @@ -4761,7 +4761,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this qemu doesn't support the " "memory-backend-ram object")); -goto cleanup; +goto cleanup; } /* report back that using the new backend is not necessary to achieve @@ -4967,7 +4967,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, if (qemuBuildDeviceAddressStr(&buf, def, &net->info, qemuCaps) < 0) goto error; if (qemuBuildRomStr(&buf, &net->info, qemuCaps) < 0) - goto error; +goto error; if (bootindex && virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) virBufferAsprintf(&buf, ",bootindex=%d", bootindex); @@ -5512,7 +5512,7 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def, if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0) goto error; if (qemuBuildRomStr(&buf, dev->info, qemuCaps) < 0) - goto error; +goto error; if (virBufferCheckError(&buf) < 0) goto error; @@ -5577,8 +5577,7 @@ qemuBuildRedirdevDevStr(virDomainDefPtr def, } virBufferAsprintf(&buf, "usb-redir,chardev=char%s,id=%s", - dev->info.alias, - dev->info.alias); + dev->info.alias, dev->info.alias); if (redirfilter && redirfilter->nusbdevs) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR_FILTER)) { @@ -9610,7 +9609,7 @@ qemuBuildCommandLine(virConnectPtr conn, } else { virCommandAddArg(cmd, "-parallel"); if (!(devstr = qemuBuildChrArgStr(¶llel->source, NULL))) - goto error; +goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); } @@ -10558,7 +10557,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("no addresses are suported for isa-serial")); + _("no addresses are supported for isa-serial")); goto error; } break; @@ -10611,7 +10610,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: if (!(*deviceStr = qemuBuildVirtioSerialPortDevStr(chr, qemuCaps))) -goto cleanup; +goto cleanup; break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE: @@ -11467,7 +11466,7 @@ qemuParseCommandLinePCI(const char *val) virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(); if (!def) - goto error; +goto error; if (!STRPREFIX(val, "host=")) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -11521,7 +11520,7 @@ qemuParseCommandLineUSB(const char *val) char *end; if (!def) - goto error; +goto error; usbsrc = &def->source.subsys.u.usb; if (!STRPREFIX(val, "host:")) { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] qemu: read backing chain names from qemu
https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that after a series of disk snapshots into existing destination images, followed by active commits of the top image, it is possible for qemu 2.2 and earlier to end up tracking a different name for the image than what it would have had when opening the chain afresh. That is, when starting with the chain 'a <- b <- c', the name associated with 'b' is how it was spelled in the metadata of 'c', but when starting with 'a', taking two snapshots into 'a <- b <- c', then committing 'c' back into 'b', the name associated with 'b' is now the name used when taking the first snapshot. Sadly, older qemu doesn't know how to treat different spellings of the same filename as identical files (it uses strcmp() instead of checking for the same inode), which means libvirt's attempt to commit an image using solely the names learned from qcow2 metadata fails with a cryptic: error: internal error: unable to execute QEMU command 'block-commit': Top image file /tmp/images/c/../b/b not found even though the file exists. Trying to teach libvirt the rules on which name qemu will expect is not worth the effort (besides, we'd have to remember it across libvirtd restarts, and track whether a file was opened via metadata or via snapshot creation for a given qemu process); it is easier to just always directly ask qemu what string it expects to see in the first place. As a safety valve, we validate that any name returned by qemu still maps to the same local file as we have tracked it, so that a compromised qemu cannot accidentally cause us to act on an incorrect file. * src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New prototype. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup): Likewise. * src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskNameLookup) (qemuMonitorJSONDiskNameLookupOne): Likewise. * src/qemu/qemu_driver.c (qemuDomainBlockCommit) (qemuDomainBlockJobImpl): Use it. Signed-off-by: Eric Blake --- v3: better sanity checks in qemu_monitor_json, rebase and retested atop Peter's fixes for interlocking block jobs src/qemu/qemu_driver.c | 28 ++-- src/qemu/qemu_monitor.c | 20 - src/qemu/qemu_monitor.h | 8 +++- src/qemu/qemu_monitor_json.c | 102 ++- src/qemu/qemu_monitor_json.h | 9 +++- 5 files changed, 149 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f4b8dab..9129531 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16283,9 +16283,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto endjob; if (baseSource) { -if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0) -goto endjob; - if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -16323,8 +16320,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, - speed, mode, async); +if (baseSource) +basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, + baseSource); +if (!baseSource || basePath) +ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, + speed, mode, async); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; if (ret < 0) { @@ -17050,12 +17051,6 @@ qemuDomainBlockCommit(virDomainPtr dom, VIR_DISK_CHAIN_READ_WRITE) < 0)) goto endjob; -if (qemuGetDriveSourceString(topSource, NULL, &topPath) < 0) -goto endjob; - -if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0) -goto endjob; - if (flags & VIR_DOMAIN_BLOCK_COMMIT_RELATIVE && topSource != disk->src) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { @@ -17086,9 +17081,14 @@ qemuDomainBlockCommit(virDomainPtr dom, disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; } qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorBlockCommit(priv->mon, device, - topPath, basePath, backingPath, - speed); +basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, + baseSource); +topPath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, +topSource); +if (basePath && topPath) +ret = qemuMonitorBlockCommit(priv->mon, device, + topPath, basePath, backingPath, +
Re: [libvirt] [PATCH] libxl: fix regression introduced by commit 4ab8cd77
Daniel P. Berrange wrote: > On Fri, Mar 13, 2015 at 06:38:24PM -0600, Jim Fehlig wrote: > >> Commit 4ab8cd77 added a check requiring input devices to have >> a bus type of VIR_DOMAIN_INPUT_BUS_USB, failing to start the >> domain otherwise. But virDomainDefParseXML adds implicit mouse >> and keyboard if a graphics device is configured. See calls to >> virDomainDefMaybeAddInput. >> >> The regression is fixed by removing the check requiring USB input >> devices, and skipping non-USB input devices when populating USB >> 'usbdevice' in libxl_domain_build_info struct. >> > > > So IIUC the problem is that we're adding an mouse + keyboard > with input bus == Xen (paravirt) or bus == ps2. (HVM). With > libxl though, these are implicitly present by default when > you have graphics enabled, so you don't want libxlMakeDomBuildInfo > to see these input devices. Yes. FWIW, the legacy xen driver behaves similarly. Only USB input devices are processed. > You only want to be looking at the > USB input devices, as they're the only ones that neeed extra > config settings procssed, is that right ? > Correct. > > >> Signed-off-by: Jim Fehlig >> --- >> src/libxl/libxl_conf.c | 11 --- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index 50ef9d8..2b57d0b 100644 >> --- a/src/libxl/libxl_conf.c >> +++ b/src/libxl/libxl_conf.c >> @@ -750,13 +750,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, >> libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0); >> >> if (def->ninputs) { >> -for (i = 0; i < def->ninputs; i++) { >> -if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) { >> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> -_("libxenlight supports only USB input")); >> -return -1; >> -} >> -} >> #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST >> if (VIR_ALLOC_N(b_info->u.hvm.usbdevice_list, def->ninputs+1) < >> 0) >> return -1; >> @@ -769,6 +762,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, >> #endif >> for (i = 0; i < def->ninputs; i++) { >> char **usbdevice; >> + >> +if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) >> +continue; >> + >> #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST >> usbdevice = &b_info->u.hvm.usbdevice_list[i]; >> #else >> > > ACK > Thanks! Will push shortly. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: fix regression introduced by commit 4ab8cd77
On Fri, Mar 13, 2015 at 06:38:24PM -0600, Jim Fehlig wrote: > Commit 4ab8cd77 added a check requiring input devices to have > a bus type of VIR_DOMAIN_INPUT_BUS_USB, failing to start the > domain otherwise. But virDomainDefParseXML adds implicit mouse > and keyboard if a graphics device is configured. See calls to > virDomainDefMaybeAddInput. > > The regression is fixed by removing the check requiring USB input > devices, and skipping non-USB input devices when populating USB > 'usbdevice' in libxl_domain_build_info struct. So IIUC the problem is that we're adding an mouse + keyboard with input bus == Xen (paravirt) or bus == ps2. (HVM). With libxl though, these are implicitly present by default when you have graphics enabled, so you don't want libxlMakeDomBuildInfo to see these input devices. You only want to be looking at the USB input devices, as they're the only ones that neeed extra config settings procssed, is that right ? > > Signed-off-by: Jim Fehlig > --- > src/libxl/libxl_conf.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index 50ef9d8..2b57d0b 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -750,13 +750,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0); > > if (def->ninputs) { > -for (i = 0; i < def->ninputs; i++) { > -if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) { > -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > -_("libxenlight supports only USB input")); > -return -1; > -} > -} > #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST > if (VIR_ALLOC_N(b_info->u.hvm.usbdevice_list, def->ninputs+1) < > 0) > return -1; > @@ -769,6 +762,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > #endif > for (i = 0; i < def->ninputs; i++) { > char **usbdevice; > + > +if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) > +continue; > + > #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST > usbdevice = &b_info->u.hvm.usbdevice_list[i]; > #else ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] networkStateInitialize: Don't lock network driver
There's no need to lock the network driver, as network driver initialization is done prior accepting any client. There's nobody to hop in and do something over partially initialized driver. Nor qemu driver is doing that. ==30532== Observed (incorrect) order is: acquisition of lock at 0x1439EF50 ==30532==at 0x4C31A26: pthread_mutex_lock (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so) ==30532==by 0x5324895: virMutexLock (virthread.c:88) ==30532==by 0x5307E86: virObjectLock (virobject.c:323) ==30532==by 0x5396440: virNetworkObjListForEach (network_conf.c:4511) ==30532==by 0x19B29308: networkStateInitialize (bridge_driver.c:686) ==30532==by 0x53E1CCC: virStateInitialize (libvirt.c:777) ==30532==by 0x11DEB7: daemonRunStateInit (libvirtd.c:906) ==30532==by 0x5324B6A: virThreadHelper (virthread.c:197) ==30532==by 0x4C30456: ??? (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so) ==30532==by 0xA1EC1F2: start_thread (in /lib64/libpthread-2.19.so) ==30532==by 0xA4EDC8C: clone (in /lib64/libc-2.19.so) ==30532== ==30532== followed by a later acquisition of lock at 0x1439CD60 ==30532==at 0x4C31A26: pthread_mutex_lock (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so) ==30532==by 0x5324895: virMutexLock (virthread.c:88) ==30532==by 0x19B27B2C: networkDriverLock (bridge_driver.c:102) ==30532==by 0x19B27B60: networkGetDnsmasqCaps (bridge_driver.c:113) ==30532==by 0x19B2856A: networkUpdateState (bridge_driver.c:389) ==30532==by 0x53963E9: virNetworkObjListForEachHelper (network_conf.c:4488) ==30532==by 0x52E2224: virHashForEach (virhash.c:521) ==30532==by 0x539645B: virNetworkObjListForEach (network_conf.c:4512) ==30532==by 0x19B29308: networkStateInitialize (bridge_driver.c:686) ==30532==by 0x53E1CCC: virStateInitialize (libvirt.c:777) ==30532==by 0x11DEB7: daemonRunStateInit (libvirtd.c:906) ==30532==by 0x5324B6A: virThreadHelper (virthread.c:197) ==30532== ==30532== Required order was established by acquisition of lock at 0x1439CD60 ==30532==at 0x4C31A26: pthread_mutex_lock (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so) ==30532==by 0x5324895: virMutexLock (virthread.c:88) ==30532==by 0x19B27B2C: networkDriverLock (bridge_driver.c:102) ==30532==by 0x19B28DF9: networkStateInitialize (bridge_driver.c:609) ==30532==by 0x53E1CCC: virStateInitialize (libvirt.c:777) ==30532==by 0x11DEB7: daemonRunStateInit (libvirtd.c:906) ==30532==by 0x5324B6A: virThreadHelper (virthread.c:197) ==30532==by 0x4C30456: ??? (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so) ==30532==by 0xA1EC1F2: start_thread (in /lib64/libpthread-2.19.so) ==30532==by 0xA4EDC8C: clone (in /lib64/libc-2.19.so) ==30532== ==30532== followed by a later acquisition of lock at 0x1439EF50 ==30532==at 0x4C31A26: pthread_mutex_lock (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so) ==30532==by 0x5324895: virMutexLock (virthread.c:88) ==30532==by 0x5307E86: virObjectLock (virobject.c:323) ==30532==by 0x538A09C: virNetworkAssignDef (network_conf.c:527) ==30532==by 0x5391EB2: virNetworkLoadState (network_conf.c:3008) ==30532==by 0x53922D4: virNetworkLoadAllState (network_conf.c:3128) ==30532==by 0x19B2929A: networkStateInitialize (bridge_driver.c:671) ==30532==by 0x53E1CCC: virStateInitialize (libvirt.c:777) ==30532==by 0x11DEB7: daemonRunStateInit (libvirtd.c:906) ==30532==by 0x5324B6A: virThreadHelper (virthread.c:197) ==30532==by 0x4C30456: ??? (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so) ==30532==by 0xA1EC1F2: start_thread (in /lib64/libpthread-2.19.so) Signed-off-by: Michal Privoznik --- src/network/bridge_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a007388..b3dcadc 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -606,7 +606,6 @@ networkStateInitialize(bool privileged, VIR_FREE(network_driver); goto error; } -networkDriverLock(network_driver); /* configuration/state paths are one of * ~/.config/libvirt/... (session/unprivileged) @@ -677,8 +676,6 @@ networkStateInitialize(bool privileged, network_driver->networkAutostartDir) < 0) goto error; -networkDriverUnlock(network_driver); - /* Update the internal status of all allegedly active * networks according to external conditions on the host * (i.e. anything that isn't stored directly in each -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] objecteventtest: Check for virNetwork* return values
Lets not give a bad example and check for return values of virNetwork* APIs called within the test. Even though it's unlikely that any API will fail, it can happen. We're connected to the test driver after all, and our API sequence is correct. So test driver should fail only in case of bug or OOM. Signed-off-by: Michal Privoznik --- tests/objecteventtest.c | 29 ++--- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c index 052dbe5..95fbfec 100644 --- a/tests/objecteventtest.c +++ b/tests/objecteventtest.c @@ -417,7 +417,7 @@ testNetworkCreateXML(const void *data) &counter, NULL); net = virNetworkCreateXML(test->conn, networkDef); -if (virEventRunDefaultImpl() < 0) { +if (!net || virEventRunDefaultImpl() < 0) { ret = -1; goto cleanup; } @@ -429,10 +429,10 @@ testNetworkCreateXML(const void *data) cleanup: virConnectNetworkEventDeregisterAny(test->conn, id); -virNetworkDestroy(net); - -virNetworkFree(net); - +if (net) { +virNetworkDestroy(net); +virNetworkFree(net); +} return ret; } @@ -455,7 +455,7 @@ testNetworkDefine(const void *data) /* Make sure the define event is triggered */ net = virNetworkDefineXML(test->conn, networkDef); -if (virEventRunDefaultImpl() < 0) { +if (!net || virEventRunDefaultImpl() < 0) { ret = -1; goto cleanup; } @@ -481,7 +481,8 @@ testNetworkDefine(const void *data) cleanup: virConnectNetworkEventDeregisterAny(test->conn, id); -virNetworkFree(net); +if (net) +virNetworkFree(net); return ret; } @@ -494,6 +495,9 @@ testNetworkStartStopEvent(const void *data) int id; int ret = 0; +if (!test->net) +return -1; + lifecycleEventCounter_reset(&counter); id = virConnectNetworkEventRegisterAny(test->conn, test->net, @@ -509,7 +513,7 @@ testNetworkStartStopEvent(const void *data) } if (counter.startEvents != 1 || counter.stopEvents != 1 || -counter.unexpectedEvents > 0) { +counter.unexpectedEvents > 0) { ret = -1; goto cleanup; } @@ -567,13 +571,16 @@ mymain(void) ret = EXIT_FAILURE; /* Define a test network */ -test.net = virNetworkDefineXML(test.conn, networkDef); +if (!(test.net = virNetworkDefineXML(test.conn, networkDef))) +ret = EXIT_FAILURE; if (virtTestRun("Network start stop events ", testNetworkStartStopEvent, &test) < 0) ret = EXIT_FAILURE; /* Cleanup */ -virNetworkUndefine(test.net); -virNetworkFree(test.net); +if (test.net) { +virNetworkUndefine(test.net); +virNetworkFree(test.net); +} virConnectClose(test.conn); virEventRemoveTimeout(timer); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] network_conf: Drop virNetworkObjIsDuplicate
This function does not make any sense now, that network driver is (almost) dropped. I mean, previously, when threads were serialized, this function was there to check, if no other network with the same name or UUID exists. However, nowadays that threads can run more in parallel, this function is useless, in fact it gives misleading return values. Consider the following scenario. Two threads, both trying to define networks with same name but different UUID (e.g. because it was generated during XML parsing phase, whatever). Lets assume that both threads are about to call networkValidate() which immediately calls virNetworkObjIsDuplicate(). T1: calls virNetworkObjIsDuplicate() and since no network with given name or UUID exist, success is returned. T2: calls virNetworkObjIsDuplicate() and since no network with given name or UUID exist, success is returned. T1: calls virNetworkAssignDef() and successfully places its network into the virNetworkObjList. T2: calls virNetworkAssignDef() and since network with the same name exists, the network definition is replaced. Okay, this is mainly because virNetworkAssignDef() does not check whether name and UUID matches. Well, lets make it so! And drop useless function too. Signed-off-by: Michal Privoznik --- src/conf/network_conf.c | 171 ++ src/conf/network_conf.h | 10 +-- src/libvirt_private.syms | 1 - src/network/bridge_driver.c | 17 ++-- src/parallels/parallels_network.c | 4 +- src/test/test_driver.c| 10 ++- 6 files changed, 99 insertions(+), 114 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index d7c5dec..1fb06ef 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -504,6 +504,81 @@ virNetworkObjAssignDef(virNetworkObjPtr network, } /* + * If flags & VIR_NETWORK_OBJ_LIST_ADD_CHECK_LIVE then this will + * refuse updating an existing def if the current def is Live + * + * If flags & VIR_NETWORK_OBJ_LIST_ADD_LIVE then the @def being + * added is assumed to represent a live config, not a future + * inactive config + */ +static virNetworkObjPtr +virNetworkAssignDefLocked(virNetworkObjListPtr nets, + virNetworkDefPtr def, + unsigned int flags) +{ +virNetworkObjPtr network; +virNetworkObjPtr ret = NULL; +char uuidstr[VIR_UUID_STRING_BUFLEN]; + +/* See if a network with matching UUID already exists */ +if ((network = virNetworkObjFindByUUIDLocked(nets, def->uuid))) { +virObjectLock(network); +/* UUID matches, but if names don't match, refuse it */ +if (STRNEQ(network->def->name, def->name)) { +virUUIDFormat(network->def->uuid, uuidstr); +virReportError(VIR_ERR_OPERATION_FAILED, + _("network '%s' is already defined with uuid %s"), + network->def->name, uuidstr); +goto cleanup; +} + +if (flags & VIR_NETWORK_OBJ_LIST_ADD_CHECK_LIVE) { +/* UUID & name match, but if network is already active, refuse it */ +if (virNetworkObjIsActive(network)) { +virReportError(VIR_ERR_OPERATION_INVALID, + _("network is already active as '%s'"), + network->def->name); +goto cleanup; +} +} + +virNetworkObjAssignDef(network, + def, + !!(flags & VIR_NETWORK_OBJ_LIST_ADD_LIVE)); +} else { +/* UUID does not match, but if a name matches, refuse it */ +if ((network = virNetworkObjFindByNameLocked(nets, def->name))) { +virObjectLock(network); +virUUIDFormat(network->def->uuid, uuidstr); +virReportError(VIR_ERR_OPERATION_FAILED, + _("network '%s' already exists with uuid %s"), + def->name, uuidstr); +goto cleanup; +} + +if (!(network = virNetworkObjNew())) + goto cleanup; + +virObjectLock(network); + +virUUIDFormat(def->uuid, uuidstr); +if (virHashAddEntry(nets->objs, uuidstr, network) < 0) +goto cleanup; + +network->def = def; +network->persistent = !(flags & VIR_NETWORK_OBJ_LIST_ADD_LIVE); +virObjectRef(network); +} + +ret = network; +network = NULL; + + cleanup: +virNetworkObjEndAPI(&network); +return ret; +} + +/* * virNetworkAssignDef: * @nets: list of all networks * @def: the new NetworkDef (will be consumed by this function iff successful) @@ -519,40 +594,14 @@ virNetworkObjAssignDef(virNetworkObjPtr network, virNetworkObjPtr virNetworkAssignDef(virNetworkObjListPtr nets, virNetworkDefPtr def, -bool live) +unsigned int flags) { virNetworkObjPtr network; -
[libvirt] [PATCH 0/3] Yet a couple of network cleanups
I've noticed these while testing the code. Michal Privoznik (3): networkStateInitialize: Don't lock network driver objecteventtest: Check for virNetwork* return values network_conf: Drop virNetworkObjIsDuplicate src/conf/network_conf.c | 171 ++ src/conf/network_conf.h | 10 +-- src/libvirt_private.syms | 1 - src/network/bridge_driver.c | 20 ++--- src/parallels/parallels_network.c | 4 +- src/test/test_driver.c| 10 ++- tests/objecteventtest.c | 29 --- 7 files changed, 117 insertions(+), 128 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: block-commit: Mark disk in block jobs only on successful command
On Mon, Mar 16, 2015 at 10:09:22 -0600, Eric Blake wrote: > On 03/16/2015 09:55 AM, Peter Krempa wrote: > > Patch 51f9f03a4ca50b070c0fbfb29748d49f583e15e1 introduces a regression > > where if a blockCommit operation fails the disk is still marked as being > > part of a block job but can't be unmarked later. > > --- > > src/qemu/qemu_driver.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > ACK. Serves me right for reviewing but not thoroughly testing the > earlier patches. Pushed; Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] qemu: Don't duplicate errors when settings stats period
On Mon, Mar 16, 2015 at 02:09:39PM +0100, Erik Skultety wrote: On 03/13/2015 05:17 PM, Martin Kletzander wrote: In order not to leave old error messages set, this patch refactors the code so the error is reported only when acted upon. The only such place already rewrites any error, so cleaning up all the error reporting in qemuMonitorSetMemoryStatsPeriod() is enough. +/** + * qemuMonitorSetMemoryStatsPeriod: + * + * This function sets balloon stats update period. + * + * Returns 0 on success and -1 on error, but does *not* set an error. + */ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, int period) { int ret = -1; VIR_DEBUG("mon=%p period=%d", mon, period); -if (!mon) { -virReportError(VIR_ERR_INVALID_ARG, "%s", - _("monitor must not be NULL")); +if (!mon) return -1; -} -if (!mon->json) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("JSON monitor is required")); +if (!mon->json) +return -1; + +if (period < 0) return -1; -} Hmm. It is a nice idea, but I guess you might have forgotten to check the actual return value in qemuProcessStart (there are actually 2 appearances in qemu_process.c) like we do in most cases. I found a piece of code (see below) where we check this correctly (so it's great you refactored this, otherwise reporting 2 identical messages would be sort of confusing) This function is called from three places. When starting a domain, when attaching to a domain and from an API that requests change to this particular value. First two calls are intentionally non-fatal and hence not acted upon, since such minor issue as setting the statistics gathering period shouldn't make domains non-startable. src/qemu/qemu_driver.c r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; if (r < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unable to set balloon driver collection period")); goto endjob; if (qemuMonitorFindBalloonObjectPath(mon, "/") == 0) { ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath, period); + +/* + * Most of the calls to this function are supposed to be + * non-fatal and the only one that should be fatal wants its + * own error message. More details for debugging will be in + * the log file. + */ +if (ret < 0) +virResetLastError(); } mon->ballooninit = true; return ret; Everything else looks fine to me, though I think that other monitor calls should meet a similar fate sometime in the future. Erik pgpcztVq1JrgX.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: block-commit: Mark disk in block jobs only on successful command
On 03/16/2015 09:55 AM, Peter Krempa wrote: > Patch 51f9f03a4ca50b070c0fbfb29748d49f583e15e1 introduces a regression > where if a blockCommit operation fails the disk is still marked as being > part of a block job but can't be unmarked later. > --- > src/qemu/qemu_driver.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) ACK. Serves me right for reviewing but not thoroughly testing the earlier patches. > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 7ca993d..4b8e104 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -17086,7 +17086,8 @@ qemuDomainBlockCommit(virDomainPtr dom, > goto endjob; > } > > -disk->blockjob = true; > +if (ret == 0) > +disk->blockjob = true; > > if (mirror) { > if (ret == 0) { > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 06/10] qemu: migration: Forbid migration with memory modules lacking info
On Fri, Mar 13, 2015 at 10:38:57 -0400, John Ferlan wrote: > > > On 03/04/2015 11:24 AM, Peter Krempa wrote: > > Make sure that libvirt has all vital information needed to reliably > > represent configuration of guest's memory devices in case of a > > migration. > > > > This patch forbids migration in case the required slot number and module > > base address are not present (failed to be loaded from qemu via > > monitor). > > --- > > src/qemu/qemu_migration.c | 14 ++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > > index 20e40aa..a31ce9a 100644 > > --- a/src/qemu/qemu_migration.c > > +++ b/src/qemu/qemu_migration.c > > @@ -2016,6 +2016,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, > > virDomainObjPtr vm, > > } > > } > > > > +/* Verify that memory device config can be transferred reliably */ > > +for (i = 0; i < def->nmems; i++) { > > +virDomainMemoryDefPtr mem = def->mems[i]; > > + > > +if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && > > +mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { > > +virReportError(VIR_ERR_OPERATION_INVALID, "%s", > > + _("domain's dimm info lacks slot ID " > > + "or base address")); > > + > > +return false; > > +} > > +} > > + > > return true; > > } > > > Would this only be possibly true for an offline migration? It would > seem an online/live migration that guest startup and continued running > of libvirtd would seemingly fill in the values. Then if libvirtd is > restarted, the cached copy of the guest with the addresses is read. If > the qemuProcessAttach code is implemented, then we have an address. This is to prevent online migration in case where qemu doesn't due to a failure report the addresses that were used for the module. > > Could this be because we 'ignore' the -2 failures in patch 5? However, > if we do, then we've never "really" added support for this > functionality. Of course if the target of the migration does have it, > then there's issues. The actual hotplug code adds another place that may potentialy fail to fill the info on a failure but will not inhibit/fail the hotplug operation as it's improbable that we could recover from that due to the fact that at the point the device was exposed to the guest. > > While what's being checked is valid and safely protects us against some > unintended mutilation and thus I'd say ACK for just the safety reasons, > I'm mostly curious as to the why. Other checks in the API seem to be > valid you are not allowed to migrate because we said you couldn't > migrate with snapshots, block job running non-USB host devices, or with > a specific CPU feature enabled. But, this seems to be - something went > wrong and we decided to ignore it, so you cannot migrate this guest. Is > there "anything" we could do to possible fill in the values so that we > don't cause failure because of some decision point in libvirt? Of course > we couldn't have already gone through this in previous reviews, but my > "short term memory" would have been unplugged ;-) > > John Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] daemon: avoid memleak when ListAll returns nothing
Commit 4f25146 (v1.2.8) managed to silence Coverity, but at the cost of a memory leak detected by valgrind: ==24129== 40 bytes in 5 blocks are definitely lost in loss record 355 of 637 ==24129==at 0x4A08B1C: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==24129==by 0x5084B8E: virReallocN (viralloc.c:245) ==24129==by 0x514D5AA: virDomainObjListExport (domain_conf.c:22200) ==24129==by 0x201227DB: qemuConnectListAllDomains (qemu_driver.c:18042) ==24129==by 0x51CC1B6: virConnectListAllDomains (libvirt-domain.c:6797) ==24129==by 0x14173D: remoteDispatchConnectListAllDomains (remote.c:1580) ==24129==by 0x121BE1: remoteDispatchConnectListAllDomainsHelper (remote_dispatch.h:1072) In short, every time a client calls a ListAll variant and asks for the resulting list, but there are 0 elements to return, we end up leaking the 1-entry array that holds the NULL terminator. What's worse, a read-only client can access these functions in a tight loop to cause libvirtd to eventually run out of memory; and this can be considered a denial of service attack against more privileged clients. Thankfully, the leak is so small (8 bytes per call) that you would already have some other denial of service with any guest calling the API that frequently, so an out-of-memory crash is unlikely enough that this did not warrant a CVE. * daemon/remote.c (remoteDispatchConnectListAllDomains) (remoteDispatchDomainListAllSnapshots) (remoteDispatchDomainSnapshotListAllChildren) (remoteDispatchConnectListAllStoragePools) (remoteDispatchStoragePoolListAllVolumes) (remoteDispatchConnectListAllNetworks) (remoteDispatchConnectListAllInterfaces) (remoteDispatchConnectListAllNodeDevices) (remoteDispatchConnectListAllNWFilters) (remoteDispatchConnectListAllSecrets) (remoteDispatchNetworkGetDHCPLeases): Plug leak. Signed-off-by: Eric Blake --- I'm pushing this now, and backporting to stable maintenance branches, based on review on the libvirt-security list (where we decided it was not severe enough to need a CVE). I'll be sending a Libvirt Security Notice about the issue later. daemon/remote.c | 55 ++- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ff64eeb..fc2237d 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1609,11 +1609,10 @@ remoteDispatchConnectListAllDomains(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); -if (doms && ndomains > 0) { +if (doms && ndomains > 0) for (i = 0; i < ndomains; i++) virObjectUnref(doms[i]); -VIR_FREE(doms); -} +VIR_FREE(doms); return rv; } @@ -4605,11 +4604,10 @@ remoteDispatchDomainListAllSnapshots(virNetServerPtr server ATTRIBUTE_UNUSED, if (rv < 0) virNetMessageSaveError(rerr); virObjectUnref(dom); -if (snaps && nsnaps > 0) { +if (snaps && nsnaps > 0) for (i = 0; i < nsnaps; i++) virObjectUnref(snaps[i]); -VIR_FREE(snaps); -} +VIR_FREE(snaps); return rv; } @@ -4674,11 +4672,10 @@ remoteDispatchDomainSnapshotListAllChildren(virNetServerPtr server ATTRIBUTE_UNU virNetMessageSaveError(rerr); virObjectUnref(snapshot); virObjectUnref(dom); -if (snaps && nsnaps > 0) { +if (snaps && nsnaps > 0) for (i = 0; i < nsnaps; i++) virObjectUnref(snaps[i]); -VIR_FREE(snaps); -} +VIR_FREE(snaps); return rv; } @@ -4733,11 +4730,10 @@ remoteDispatchConnectListAllStoragePools(virNetServerPtr server ATTRIBUTE_UNUSED cleanup: if (rv < 0) virNetMessageSaveError(rerr); -if (pools && npools > 0) { +if (pools && npools > 0) for (i = 0; i < npools; i++) virObjectUnref(pools[i]); -VIR_FREE(pools); -} +VIR_FREE(pools); return rv; } @@ -4796,11 +4792,10 @@ remoteDispatchStoragePoolListAllVolumes(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); -if (vols && nvols > 0) { +if (vols && nvols > 0) for (i = 0; i < nvols; i++) virObjectUnref(vols[i]); -VIR_FREE(vols); -} +VIR_FREE(vols); virObjectUnref(pool); return rv; } @@ -4856,11 +4851,10 @@ remoteDispatchConnectListAllNetworks(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); -if (nets && nnets > 0) { +if (nets && nnets > 0) for (i = 0; i < nnets; i++) virObjectUnref(nets[i]); -VIR_FREE(nets); -} +VIR_FREE(nets); return rv; } @@ -4915,11 +4909,10 @@ remoteDispatchConnectListAllInterfaces(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); -if (ifaces && nifaces > 0) { +if (ifaces && nifaces > 0) for (i =
Re: [libvirt] [PATCH 00/12] More cleanup from IOThreads changes
On 03/13/2015 11:11 PM, John Ferlan wrote: > During the review process a few things were pointed at as perhaps > needing some adjustments based on what was done for IOThreads. > Specifically a memory leak in PinVcpuFlags since PinIOThreads was > just a copy of the Vcpu code and secondarily since the IOThreads > code "reused" the virDomainVcpuPin* data structures and API's, those > should change names to be more generic. > > > John Ferlan (12): > qemu: Fix possible memory leak in qemuDomainPinVcpuFlags > Convert virDomainVcpuPinDefPtr to virDomainPinDefPtr > Convert virDomainPinDefPtr->vcpuid to virDomainPinDefPtr->id > Convert virDomainVcpuPinDefFree to virDomainPinDefFree > Convert virDomainVcpuPinDefArrayFree to virDomainPinDefArrayFree > Convert virDomainVcpuPinDefCopy into virDomainPinDefCopy > Convert virDomainVcpuPinIsDuplicate into virDomainPinIsDuplicate > Convert virDomainVcpuPinFindByVcpu into virDomainPinFindByVcpu > Replace virDomainVcpuPinAdd with virDomainPinAdd > Replace virDomainIOThreadsPinAdd with virDomainPinAdd > Replace virDomainVcpuPinDel with virDomainPinDel > Remove virDomainIOThreadsPinDel > > src/conf/domain_conf.c | 234 > +-- > src/conf/domain_conf.h | 58 +--- > src/libvirt_private.syms | 16 ++-- > src/libxl/libxl_domain.c | 2 +- > src/libxl/libxl_driver.c | 18 ++-- > src/qemu/qemu_cgroup.c | 12 +-- > src/qemu/qemu_cgroup.h | 4 +- > src/qemu/qemu_driver.c | 104 +++-- > src/qemu/qemu_process.c | 16 ++-- > src/xen/xend_internal.c | 10 +- > 10 files changed, 204 insertions(+), 270 deletions(-) > Cleaned up patch 8 (virDomainVcpuPinFindByVcpu into virDomainPinFindByVcpu) to be virDomainVcpuPinFindByVcpu into virDomainPinFind Adjusted the terse commit comments a bit and pushed Thanks - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: block-commit: Mark disk in block jobs only on successful command
Patch 51f9f03a4ca50b070c0fbfb29748d49f583e15e1 introduces a regression where if a blockCommit operation fails the disk is still marked as being part of a block job but can't be unmarked later. --- src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7ca993d..4b8e104 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17086,7 +17086,8 @@ qemuDomainBlockCommit(virDomainPtr dom, goto endjob; } -disk->blockjob = true; +if (ret == 0) +disk->blockjob = true; if (mirror) { if (ret == 0) { -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 08/10] qemu: conf: Add support for memory device cold(un)plug
On Fri, Mar 13, 2015 at 10:39:05 -0400, John Ferlan wrote: > On 03/04/2015 11:24 AM, Peter Krempa wrote: > > Add a few helpers that allow to operate with memory device definitions > > on the domain config and use them to implement memory device coldplug in > > the qemu driver. > > --- > > src/conf/domain_conf.c | 100 > > +++ > > src/conf/domain_conf.h | 10 + > > src/libvirt_private.syms | 4 ++ > > src/qemu/qemu_driver.c | 15 ++- > > 4 files changed, 127 insertions(+), 2 deletions(-) > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > index 4f20aa6..0f6058b 100644 > > --- a/src/conf/domain_conf.c > > +++ b/src/conf/domain_conf.c > > @@ -12687,6 +12687,106 @@ virDomainRNGRemove(virDomainDefPtr def, > > } > > > > > > +static int > > +virDomainMemoryFindByDefInternal(virDomainDefPtr def, > > + virDomainMemoryDefPtr mem, > > + bool allowAddressFallback) > > +{ > > +size_t i; > > + > > +for (i = 0; i < def->nmems; i++) { > > +virDomainMemoryDefPtr tmp = def->mems[i]; > > + > > +/* address, if present */ > > +if (allowAddressFallback) { > > +if (tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) > > +continue; > > So this path says - we want address fallback and this device has either > DIMM or LAST (oy!) as a type, so go to the next one and ignore this one or any other type as PCI, USB ... basically the check is here as if the device had any address assigned it should have been rejected already by previous call to this function with @allowAddressFallback equal to false. > > > +} else { > > +if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && > > +!virDomainDeviceInfoAddressIsEqual(&tmp->info, &mem->info)) > > +continue; > > This path says - we don't want address fallback and as long as we're > DIMM or LAST (oy!), then compare ... or any other supported address type which translates to "the address is set" > > What happens when we add a new address type? It would seem the more > straightforward way would be > > if (type == DIMM && !Equal) > if (!allowAddressFallback) > continue' No, it would actually make it less straightforward. This code is shared so it has to be device address type agnostic. > > Otherwise we're falling through to alias, target, etc. checks > > > +} > > + > > +/* alias, if present */ > > +if (mem->info.alias && > > +STRNEQ_NULLABLE(tmp->info.alias, mem->info.alias)) > > +continue; > > I thought the NULLABLE checks both elems for NULL... It does indeed, but in this case the logic of the lookup requires the extra check as if the user doesn't specify the alias of the memory device to find, we still do might want do find a device definition that already has the alias filled but the user didn't specify it explicitly. > > > + > > +/* target info -> always present */ > > +if (tmp->model != mem->model || > > +tmp->targetNode != mem->targetNode || > > +tmp->size != mem->size) > > +continue; > > + > > +/* source stuff -> match with device */ > > +if (tmp->pagesize != mem->pagesize) > > +continue; > > + > > +if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes)) > > +continue; > > + > > +break; > > +} > > + > > +if (i == def->nmems) > > +return -1; > > + > > +return i; > > +} > > + > > + > > +int > > +virDomainMemoryFindByDef(virDomainDefPtr def, > > + virDomainMemoryDefPtr mem) > > +{ > > +return virDomainMemoryFindByDefInternal(def, mem, false); > > +} > > + > > + > > +int > > +virDomainMemoryFindInactiveByDef(virDomainDefPtr def, > > + virDomainMemoryDefPtr mem) > > +{ > > +int ret; > > + > > +if ((ret = virDomainMemoryFindByDefInternal(def, mem, false)) < 0) > > +ret = virDomainMemoryFindByDefInternal(def, mem, true); > > I would seem Inactive would probably not have the dimm address set, so > we're incurring a 2X penalty for perhaps no reason... Unless perhaps the > incoming mem def being checked has an address... The address will be set if and only if the user provided it when adding the device. Ohterwise the address will be empty. In that case we want to find the correct device first and ignore addresses in case the user provided the XML from the running device but is expecting the inactive configuration (which does not have an address set) to be removed too. > > That is if my incoming has an address, then don't allow fallback; > otherwise, well best match. That exactly wouldn't work in my previous example, where you take the live XML (with the addres auto-assigned) the code would not match it's original definition that did not
Re: [libvirt] [PATCHv2.5 09/10] qemu: Implement memory device hotplug
On Fri, Mar 13, 2015 at 20:48:39 -0400, John Ferlan wrote: > > > On 03/04/2015 11:25 AM, Peter Krempa wrote: > > Add code to hot-add memory devices to running qemu instances. > > --- > > src/qemu/qemu_command.c | 4 +-- > > src/qemu/qemu_command.h | 15 + > > src/qemu/qemu_driver.c | 5 ++- > > src/qemu/qemu_hotplug.c | 85 > > + > > src/qemu/qemu_hotplug.h | 3 ++ > > 5 files changed, 109 insertions(+), 3 deletions(-) ... > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > > index 6b10e96..b917d76 100644 > > --- a/src/qemu/qemu_driver.c > > +++ b/src/qemu/qemu_driver.c > > @@ -7095,8 +7095,11 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, > > dev->data.rng = NULL; > > break; > > > > -/*TODO: implement later */ > > case VIR_DOMAIN_DEVICE_MEMORY: > > +ret = qemuDomainAttachMemory(driver, vm, > > + dev->data.memory); > > Shouldn't there be a : > >if (!ret) > > prior to the following line (like the other cases in the switch) No as qemuDomainAttachMemory always consumes the memory device definition. I'll add a comment in the next version to clarify that. > > > +dev->data.memory = NULL; > > +break; > > > > case VIR_DOMAIN_DEVICE_NONE: > > case VIR_DOMAIN_DEVICE_FS: > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > > index cb2270e..967b678 100644 > > --- a/src/qemu/qemu_hotplug.c > > +++ b/src/qemu/qemu_hotplug.c > > @@ -1672,6 +1672,91 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, > > } > > > > > > +int > > +qemuDomainAttachMemory(virQEMUDriverPtr driver, > > + virDomainObjPtr vm, > > + virDomainMemoryDefPtr mem) > > +{ > > +qemuDomainObjPrivatePtr priv = vm->privateData; > > +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > > +char *devstr = NULL; > > +char *objalias = NULL; > > +const char *backendType; > > +virJSONValuePtr props = NULL; > > +int id; > > +int ret = -1; > > + > > +if (virAsprintf(&mem->info.alias, "dimm%zu", vm->def->nmems) < 0) > > +goto cleanup; > > + > > +if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0) > > +goto cleanup; > > + > > +if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv->qemuCaps))) > > +goto cleanup; > > + > > +qemuDomainMemoryDeviceAlignSize(mem); > > + > > +if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize, > > + mem->targetNode, mem->sourceNodes, NULL, > > + vm->def, priv->qemuCaps, cfg, > > + &backendType, &props, true) < 0) > > +goto cleanup; > > + > > +if (virDomainMemoryInsert(vm->def, mem) < 0) { > > +virJSONValueFree(props); > > +goto cleanup; > > +} > > + > > +qemuDomainObjEnterMonitor(driver, vm); > > +if (qemuMonitorAddObject(priv->mon, backendType, objalias, props) < 0) > > I see from the AddObject comments, props is consumed upon success, but > if we hit the else of mon->json, then we don't free it which we should - > not related to this particular code, but something that should be fixed... I'll fixed that as a separate patch that will be part of the next posting. > > +goto removedef; > > + > > +if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { > > +virErrorPtr err = virSaveLastError(); > > +ignore_value(qemuMonitorDelObject(priv->mon, objalias)); > > +virSetError(err); > > +virFreeError(err); > > +goto removedef; > > +} > > + > > +if (qemuDomainObjExitMonitor(driver, vm) < 0) { > > +/* we shouldn't touch mem now, as the def might be freed */ > > +mem = NULL; > > +goto cleanup; > > +} > > + > > +/* mem is consumed by vm->def */ > > +mem = NULL; > > Since both the Exit error path and this path set mem = NULL, why not > just set it before the Exit and comment that it'll either consumed by > vm->def on success or might be freed on failure... It's always consumed, either freed or added to the definition. In some cases it's necessary to track it separately as the @vm was unlocked during monitor access. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH python v10] Expose virDomainInterfacesAddresses to python binding
From: Nehal J Wani examples/Makefile.am: * Add new file domipaddrs.py examples/README: * Add documentation for the python example libvirt-override-api.xml: * Add new symbol for virDomainInterfacesAddresses libvirt-override.c: * Hand written python api Example: $ python examples/domipaddrs.py qemu:///system f18 Interface MAC address Protocol Address lo 00:00:00:00:00:00ipv4 127.0.0.1/8 lo 00:00:00:00:00:00ipv6 ::1/128 eth3 52:54:00:20:70:3dipv4 192.168.105.240/16 eth3 52:54:00:20:70:3dipv6 fe80::5054:ff:fe20:703d/64 eth2 52:54:00:36:2a:e5N/A N/A eth1 52:54:00:b1:70:19ipv4 192.168.105.201/16 eth1 52:54:00:b1:70:19ipv4 192.168.201.195/16 eth1 52:54:00:b1:70:19ipv6 fe80::5054:ff:feb1:7019/64 eth0 52:54:00:2e:45:ceipv4 10.1.33.188/24 eth0 52:54:00:2e:45:ceipv6 2001:db8:0:f101::2/64 eth0 52:54:00:2e:45:ceipv6 fe80::5054:ff:fe2e:45ce/64 --- MANIFEST.in | 1 + examples/README | 1 + examples/domipaddrs.py | 57 + generator.py | 2 + libvirt-override-api.xml | 9 +++- libvirt-override.c | 105 +++ sanitytest.py| 3 ++ 7 files changed, 177 insertions(+), 1 deletion(-) create mode 100755 examples/domipaddrs.py diff --git a/MANIFEST.in b/MANIFEST.in index d7bc545..dd05221 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -4,6 +4,7 @@ include COPYING include COPYING.LESSER include ChangeLog include examples/consolecallback.py +include examples/domipaddrs.py include examples/dominfo.py include examples/domrestore.py include examples/domsave.py diff --git a/examples/README b/examples/README index 5b5d405..1d4b425 100644 --- a/examples/README +++ b/examples/README @@ -11,6 +11,7 @@ domrestore.py - restore domU's from their saved files in a directory esxlist.py - list active domains of an VMware ESX host and print some info. also demonstrates how to use the libvirt.openAuth() method dhcpleases.py - list dhcp leases for a given virtual network +domipaddrs.py - list IP addresses for guest domains The XML files in this directory are examples of the XML format that libvirt expects, and will have to be adapted for your setup. They are only needed diff --git a/examples/domipaddrs.py b/examples/domipaddrs.py new file mode 100755 index 000..d6d5cac --- /dev/null +++ b/examples/domipaddrs.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python +# domipaddrs - print domain interfaces along with their MAC and IP addresses + +import libvirt +import sys + +def usage(): +print "Usage: %s [URI] DOMAIN" % sys.argv[0] +print "Print domain interfaces along with their MAC and IP addresses" + +uri = None +name = None +args = len(sys.argv) + +if args == 2: +name = sys.argv[1] +elif args == 3: +uri = sys.argv[1] +name = sys.argv[2] +else: +usage() +sys.exit(2) + +conn = libvirt.open(uri) +if conn == None: +print "Unable to open connection to libvirt" +sys.exit(1) + +try: +dom = conn.lookupByName(name) +except libvirt.libvirtError: +print "Domain %s not found" % name +sys.exit(0) + +ifaces = dom.interfaceAddresses(libvirt.VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE); +if (ifaces == None): +print "Failed to get domain interfaces" +sys.exit(0) + +print " {0:10} {1:20} {2:12} {3}".format("Interface", "MAC address", "Protocol", "Address") + +def toIPAddrType(addrType): +if addrType == libvirt.VIR_IP_ADDR_TYPE_IPV4: +return "ipv4" +elif addrType == libvirt.VIR_IP_ADDR_TYPE_IPV6: +return "ipv6" + +for (name, val) in ifaces.iteritems(): +if val['addrs']: +for addr in val['addrs']: + print " {0:10} {1:19}".format(name, val['hwaddr']), + print " {0:12} {1}/{2} ".format(toIPAddrType(addr['type']), addr['addr'], addr['prefix']), + print +else: +print " {0:10} {1:19}".format(name, val['hwaddr']), +print " {0:12} {1}".format("N/A", "N/A"), +print diff --git a/generator.py b/generator.py index df7a74d..8c1c48e 100755 --- a/generator.py +++ b/generator.py @@ -483,6 +483,7 @@ skip_impl = ( 'virDomainBlockCopy', 'virNodeAllocPages', 'virDomainGetFSInfo', +'virDomainInterfaceAddresses', ) lxc_skip_impl = ( @@ -595,6 +596,7 @@ skip_function = ( 'virDomainStatsRecordListFree', # only useful in C, python uses dict 'virDomainFSInfoFree', # only useful in C, python code uses list 'virDomainIOThreadsInfoFree', # only useful in C, python code uses list +'virDomainInterfaceFree', # only useful in C, python code uses list ) lxc_skip_function = ( diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml index 4660c9f..b197639 100644 --- a/l
Re: [libvirt] [PATCH v10 3/4] domifaddr: Implement the API for qemu
On Mon, Mar 16, 2015 at 08:42:49AM -0400, John Ferlan wrote: > > > On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > > From: Nehal J Wani > > > > By querying the qemu guest agent with the QMP command > > "guest-network-get-interfaces" and converting the received JSON > > output to structured objects. > > > > Although "ifconfig" is deprecated, IP aliases created by "ifconfig" > > are supported by this API. The legacy syntax of an IP alias is: > > ":". Since we want all aliases to be clubbed > > under parent interface, simply stripping ":" suffices. > > Note that IP aliases formed by "ip" aren't visible to "ifconfig", > > and aliases created by "ip" do not have any specific name. But > > we are lucky, as qemu guest agent detects aliases created by both. > > > > src/qemu/qemu_agent.h: > > * Define qemuAgentGetInterfaces > > > > src/qemu/qemu_agent.c: > > * Implement qemuAgentGetInterface > > > > src/qemu/qemu_driver.c: > > * New function qemuGetDHCPInterfaces > > * New function qemuDomainInterfaceAddresses > > > > src/remote_protocol-sructs: > > * Define new structs > > > > tests/qemuagenttest.c: > > * Add new test: testQemuAgentGetInterfaces > > Test cases for IP aliases, 0 or multiple ipv4/ipv6 address(es) > > > > Signed-off-by: Nehal J Wani > > --- > > src/qemu/qemu_agent.c | 204 > > + > > src/qemu/qemu_agent.h | 4 + > > src/qemu/qemu_driver.c | 175 ++ > > tests/qemuagenttest.c | 188 + > > 4 files changed, 571 insertions(+) > > > > ACK - there's a NIT and a comment that follows, I'll let you decide how > to handle. > > John > > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c > > index 5fcc40f..8155006 100644 > > --- a/src/qemu/qemu_agent.c > > +++ b/src/qemu/qemu_agent.c > > @@ -1953,3 +1953,207 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, > > virDomainFSInfoPtr **info, > > virJSONValueFree(reply); > > return ret; > > } > > + > > +/* > > + * qemuAgentGetInterfaces: > > + * @mon: Agent monitor > > + * @ifaces: pointer to an array of pointers pointing to interface objects > > + * > > + * Issue guest-network-get-interfaces to guest agent, which returns a > > + * list of interfaces of a running domain along with their IP and MAC > > + * addresses. > > + * > > + * Returns: number of interfaces on success, -1 on error. > > + */ > > +int > > +qemuAgentGetInterfaces(qemuAgentPtr mon, > > + virDomainInterfacePtr **ifaces) > > +{ > > +int ret = -1; > > +size_t i, j; > > +int size = -1; > > +virJSONValuePtr cmd = NULL; > > +virJSONValuePtr reply = NULL; > > +virJSONValuePtr ret_array = NULL; > > +size_t ifaces_count = 0; > > +size_t addrs_count = 0; > > +virDomainInterfacePtr *ifaces_ret = NULL; > > +virHashTablePtr ifaces_store = NULL; > > +char **ifname = NULL; > > + > > +/* Hash table to handle the interface alias */ > > +if (!(ifaces_store = virHashCreate(ifaces_count, NULL))) { > > +virHashFree(ifaces_store); > > +return -1; > > +} > > + > > +if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", > > NULL))) > > +goto cleanup; > > + > > +if (qemuAgentCommand(mon, cmd, &reply, false, > > VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 || > > +qemuAgentCheckError(cmd, reply) < 0) { > > +goto cleanup; > > +} > > + > > +if (!(ret_array = virJSONValueObjectGet(reply, "return"))) { > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > + _("qemu agent didn't provide 'return' field")); > > +goto cleanup; > > +} > > + > > +if ((size = virJSONValueArraySize(ret_array)) < 0) { > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > + _("qemu agent didn't return an array of > > interfaces")); > > +goto cleanup; > > +} > > + > > +for (i = 0; i < size; i++) { > > +virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i); > > +virJSONValuePtr ip_addr_arr = NULL; > > +const char *hwaddr, *ifname_s, *name = NULL; > > +int ip_addr_arr_size; > > +virDomainInterfacePtr iface = NULL; > > + > > +/* Shouldn't happen but doesn't hurt to check neither */ > > +if (!tmp_iface) { > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > + _("something has went really wrong")); > > +goto error; > > +} > > + > > +/* interface name is required to be presented */ > > +name = virJSONValueObjectGetString(tmp_iface, "name"); > > +if (!name) { > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > + _("qemu agent didn't provide 'name' field")); > > +goto error; > > +} > > + > > +/* Handle interface alias (:) */ > > +ifname
Re: [libvirt] [PATCH v10 1/4] domifaddr: Implement the public APIs
On Mon, Mar 16, 2015 at 08:42:18AM -0400, John Ferlan wrote: > > > On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > > From: Nehal J Wani > > > > Define helper function virDomainInterfaceFree, which allows > > the upper layer application to free the domain interface object > > conveniently. > > > > The API is going to provide multiple methods by flags, e.g. > > * Query guest agent > > * Parse DHCP lease file > > > > include/libvirt/libvirt-domain.h > > * Define virDomainInterfaceAddresses, virDomainInterfaceFree > > * Define structs virDomainInterface, virDomainIPAddress > > > > src/driver-hypervisor.h: > > * Define domainInterfaceAddresses > > > > src/libvirt-domain.c: > > * Implement virDomainInterfaceAddresses > > * Implement virDomainInterfaceFree > > > > src/libvirt_public.syms: > > * Export the new symbols > > > > Signed-off-by: Nehal J Wani > > --- > > include/libvirt/libvirt-domain.h | 32 ++ > > src/driver-hypervisor.h | 6 ++ > > src/libvirt-domain.c | 123 > > +++ > > src/libvirt_public.syms | 2 + > > 4 files changed, 163 insertions(+) > > > > + * for (i = 0; i < ifaces_count; i++) > > + * virDomainInterfaceFree(ifaces[i]); > > + * free(ifaces); > > + * > > + * Returns the number of interfaces on success, -1 in case of error. > > + */ > > +int > > +virDomainInterfaceAddresses(virDomainPtr dom, > > +virDomainInterfacePtr **ifaces, > > +unsigned int source, > > +unsigned int flags) > > +{ > > +virConnectPtr conn; > > + > > +VIR_DOMAIN_DEBUG(dom, "ifaces=%p, source=%d, flags=%x", ifaces, > > source, flags); > > + > > +virResetLastError(); > > + > > +conn = dom->conn; > > If 'dom' is NULL, then there's a pointer deref (considering check below) > > > + > > +if (ifaces) > > +*ifaces = NULL; > > +virCheckDomainReturn(dom, -1); > > +virCheckNonNullArgGoto(ifaces, error); > > +virCheckReadOnlyGoto(conn->flags, error); > > I think I ran into this when I was adding the libvirt-perl code - since > this is a get interface - does it really matter if it's readonly? In this case it matters because we need to block access to the QEMU guest agent. > > Also, I'd move both conn = dom->conn and "if (ifaces) *ifaces = NULL;" > to here I'll just delete the 'conn' var - it is fairly pointless and just causes problems. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3.5 0/4] Automaticaly fill element for NUMA enabled guests
On Thu, Mar 12, 2015 at 10:07:04 -0400, John Ferlan wrote: > On 03/06/2015 09:40 AM, Peter Krempa wrote: > > Pavel's series changed few same places thus the previous version no longer > > applies. > > > > Peter Krempa (4): > > conf: Replace access to def->mem.max_balloon with accessor functions > > qemu: command: Add helper to align memory sizes > > conf: Automatically use NUMA memory size in case NUMA is enabled > > conf: Make specifying optional > > > > docs/schemas/domaincommon.rng | 18 ++--- > > src/conf/domain_conf.c | 81 > > +++--- > > src/conf/domain_conf.h | 4 ++ > > src/hyperv/hyperv_driver.c | 2 +- > > src/libvirt_private.syms | 3 + > > src/libxl/libxl_conf.c | 2 +- > > src/libxl/libxl_driver.c | 8 +-- > > src/lxc/lxc_cgroup.c | 2 +- > > src/lxc/lxc_driver.c | 12 ++-- > > src/lxc/lxc_fuse.c | 4 +- > > src/lxc/lxc_native.c | 4 +- > > src/openvz/openvz_driver.c | 2 +- > > src/parallels/parallels_driver.c | 2 +- > > src/parallels/parallels_sdk.c | 12 ++-- > > src/phyp/phyp_driver.c | 11 +-- > > src/qemu/qemu_command.c| 23 +++--- > > src/qemu/qemu_domain.c | 21 ++ > > src/qemu/qemu_domain.h | 2 + > > src/qemu/qemu_driver.c | 21 +++--- > > src/qemu/qemu_hotplug.c| 8 ++- > > src/qemu/qemu_process.c| 2 +- > > src/test/test_driver.c | 8 +-- > > src/uml/uml_driver.c | 8 +-- > > src/vbox/vbox_common.c | 4 +- > > src/vmware/vmware_driver.c | 2 +- > > src/vmx/vmx.c | 12 ++-- > > src/xen/xm_internal.c | 14 ++-- > > src/xenapi/xenapi_driver.c | 2 +- > > src/xenapi/xenapi_utils.c | 4 +- > > src/xenconfig/xen_common.c | 8 ++- > > src/xenconfig/xen_sxpr.c | 9 +-- > > .../qemuxml2argv-cpu-numa-no-memory-element.args | 7 ++ > > .../qemuxml2argv-cpu-numa-no-memory-element.xml| 24 +++ > > .../qemuxml2argv-minimal-no-memory.xml | 25 +++ > > .../qemuxml2argv-numatune-memnode.args | 2 +- > > tests/qemuxml2argvtest.c | 2 + > > .../qemuxml2xmlout-cpu-numa-no-memory-element.xml | 28 > > tests/qemuxml2xmltest.c| 1 + > > 38 files changed, 299 insertions(+), 105 deletions(-) > > create mode 100644 > > tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-no-memory-element.args > > create mode 100644 > > tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-no-memory-element.xml > > create mode 100644 > > tests/qemuxml2argvdata/qemuxml2argv-minimal-no-memory.xml > > create mode 100644 > > tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-no-memory-element.xml > > > > Since this is the order of your repo on git://pipo.sk/pipo/libvirt.git, > I'll start with these. > > Couple of general comments... > > - I think the new function names are better > > - I think one can tell that the "KiB" or "MiB" was realized at some > point in time after "KB" and "MB" - as there's stray comments and > variables using the (I guess) now incorrect terminology. Not that I'm > asking for them to be changed, just an "observation". > > - Whether they snuck in after you started this - there are a few direct > references to mem.max_balloon in the bhyve_command.c. Should they be > replaced by the new accessors? I actually missed fixing the bhyve driver. I've done it now. > > - Because it became apparent in patch 4... In virDomainDefParseXML, > just to be "complete" shouldn't the: > > /* Extract domain memory */ > if (virDomainParseMemory("./memory[1]", NULL, ctxt, > &def->mem.max_balloon, false, true) < 0) > > be replaced with something like: > > unsigned long long memory_value; > ... > /* Extract domain memory */ > if (virDomainParseMemory("./memory[1]", NULL, ctxt, > &memory_value, false, true) < 0) { > ... > } > virDomainDefSetMemoryInitial(def, memory_value); > > > Although it seems like overkill - it's just making sure no one tries to > cut-copy-paste in the future. In the config the implementation is actually "private" and since it's only a single place we should be okay here. > > - With having virDomainDefGetMemoryInitial u
Re: [libvirt] [PATCH] rpm-build: use pkg-config to detect wireshark presence
On Mon, Mar 16, 2015 at 02:38:40PM +0100, Pavel Hrdina wrote: > On Mon, Mar 16, 2015 at 12:46:04PM +, Daniel P. Berrange wrote: > > On Mon, Mar 16, 2015 at 01:41:15PM +0100, Pavel Hrdina wrote: > > > Wireshark supports pkg-config since 1.11.3. Right now we build > > > wireshark-dissectior tool as default trough rpm build only on > > > fedora >= 21 and there is newer wireshark that supports pkg-config. > > > If someone wants to build libvirt with wireshark-dissector against older > > > wireshark, they should specify the location by hand. > > > > > > This patch is mainly to fix wrong dependency on wireshark binary as it > > > doesn't make sense to require that binary file to just get version info > > > of that package in makefile. > > > > > > Signed-off-by: Pavel Hrdina > > > > Be nice to go one step further and switch over to using LIBVIRT_CHECK_PKG > > and having the code in m4/virt-wireshark.m4. Just copy virt-fuse.m4 as a > > equivalently simple module > > > > At first I thought that I'll be nice, but this is not that simple module. > Wireshark requires glib-2.0 and also there is an optional argument > --with-ws-plugindir. This is actually the only thing I tried to avoid, but > you're right that if I'm touching this code that it would be nice to make a > separate m4 module. I'll send a v2. You shouldn't need to check for glib2.0 yourself - if that dependancy is declared in the wireshark.pc (as it should be), then pkg-config itself will check that for you. Moving the --with-ws-plugindir arg declaration into the virt-wireshark.m4 file is pretty straightforward - we do that in a number of cases already so just checkout some more existing examples. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpm-build: use pkg-config to detect wireshark presence
On Mon, Mar 16, 2015 at 12:46:04PM +, Daniel P. Berrange wrote: > On Mon, Mar 16, 2015 at 01:41:15PM +0100, Pavel Hrdina wrote: > > Wireshark supports pkg-config since 1.11.3. Right now we build > > wireshark-dissectior tool as default trough rpm build only on > > fedora >= 21 and there is newer wireshark that supports pkg-config. > > If someone wants to build libvirt with wireshark-dissector against older > > wireshark, they should specify the location by hand. > > > > This patch is mainly to fix wrong dependency on wireshark binary as it > > doesn't make sense to require that binary file to just get version info > > of that package in makefile. > > > > Signed-off-by: Pavel Hrdina > > Be nice to go one step further and switch over to using LIBVIRT_CHECK_PKG > and having the code in m4/virt-wireshark.m4. Just copy virt-fuse.m4 as a > equivalently simple module > At first I thought that I'll be nice, but this is not that simple module. Wireshark requires glib-2.0 and also there is an optional argument --with-ws-plugindir. This is actually the only thing I tried to avoid, but you're right that if I'm touching this code that it would be nice to make a separate m4 module. I'll send a v2. Thanks for review. Pavel > > Regards, > Daniel > -- > |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] AUTHORS.in: added myself into commiters list
On 03/16/2015 01:38 PM, Peter Krempa wrote: > On Mon, Mar 16, 2015 at 09:34:56 +0100, Erik Skultety wrote: > > The actual patch is missing :) > > Peter > > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Oops, sent a regular patch to list. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] AUTHORS: add myself to commiters list
--- AUTHORS.in | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.in b/AUTHORS.in index 329924f..6673b3f 100644 --- a/AUTHORS.in +++ b/AUTHORS.in @@ -16,6 +16,7 @@ Daniel Berrange Daniel Veillard Doug Goldstein Eric Blake +Erik Skultety Gao Feng Guido G??nther J??n Tomko -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] AUTHORS: add myself to commiters list
On 03/16/2015 02:19 PM, Erik Skultety wrote: > --- > AUTHORS.in | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/AUTHORS.in b/AUTHORS.in > index 329924f..6673b3f 100644 > --- a/AUTHORS.in > +++ b/AUTHORS.in > @@ -16,6 +16,7 @@ Daniel Berrange > Daniel Veillard > Doug Goldstein > Eric Blake > +Erik Skultety > Gao Feng > Guido Günther > Ján Tomko > > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Now pushed as trivial. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] network: Resolve Coverity FORWARD_NULL
The following is a long winded way to say this patch is avoiding a false positive. Coverity complains that calling networkPlugBandwidth() could eventually end up with a NULL dereference on iface->bandwidth because in the networkAllocateActualDevice there's a check of 'iface->bandwidth' before deciding to try to use the 'portgroup' if it exists or to not perferm the virNetDevBandwidthCopy if 'bandwidth' is not NULL. Later in networkPlugBandwidth the 'iface->bandwidth' is sourced from virDomainNetGetActualBandwidth - which would be either iface->bandwidth or (preferably) iface->data.network.actual->bandwidth which would have been filled in from either 'iface->bandwidth' or 'portgroup->bandwidth' back in networkAllocateActualDevice There *is* a check in networkCheckBandwidth for the result of the virDomainNetGetActualBandwidth being NULL and a return 1 based on that which would cause networkPlugBandwidth to exit properly and thus never hit the condition that Coverity complains about. However, since Coverity checks all paths - it somehow believes that a return of 0 by networkCheckBandwidth in this condition would end up causing the possible NULL dereference. The "fix" to silence Coverity is to not have networkCheckBandwidth also call virDomainNetGetActualBandwidth in order to get the ifaceBand, but rather have it accept it as an argument which causes Coverity to "see" that it's the exit condition of 1 that won't have the possible NULL dereference. Since we're passing that, I added the passing of iface->mac rather than passing iface as well. This just hopefully makes sure someone doesn't undo this in the future... Signed-off-by: John Ferlan --- src/network/bridge_driver.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a007388..d885aa9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3820,7 +3820,7 @@ networkAllocateActualDevice(virDomainDefPtr dom, (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) || (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE)) { /* for these forward types, the actual net type really *is* - *NETWORK; we just keep the info from the portgroup in + * NETWORK; we just keep the info from the portgroup in * iface->data.network.actual */ iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK; @@ -4593,17 +4593,17 @@ networkGetNetworkAddress(const char *netname, char **netaddr) */ static int networkCheckBandwidth(virNetworkObjPtr net, - virDomainNetDefPtr iface, + virNetDevBandwidthPtr ifaceBand, + virMacAddr ifaceMac, unsigned long long *new_rate) { int ret = -1; virNetDevBandwidthPtr netBand = net->def->bandwidth; -virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); unsigned long long tmp_floor_sum = net->floor_sum; unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; -virMacAddrFormat(&iface->mac, ifmac); +virMacAddrFormat(&ifaceMac, ifmac); if (ifaceBand && ifaceBand->in && ifaceBand->in->floor && !(netBand && netBand->in)) { @@ -4689,7 +4689,8 @@ networkPlugBandwidth(virNetworkObjPtr net, char ifmac[VIR_MAC_STRING_BUFLEN]; virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); -if ((plug_ret = networkCheckBandwidth(net, iface, &new_rate)) < 0) { +if ((plug_ret = networkCheckBandwidth(net, ifaceBand, + iface->mac, &new_rate)) < 0) { /* helper reported error */ goto cleanup; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] conf: Use correct type for balloon stats period
On 03/13/2015 05:17 PM, Martin Kletzander wrote: > We're parsing memballoon status period as unsigned int, but when we're > trying to set it, both we and qemu use signed int. That means large > values will get wrapped around to negative one resulting in error. > Basically the same problem as commit e3a7b874 was dealing with when > updating live domain. > > QEMU changed the accepted value to int64 in commit 1f9296b5, but even > values as INT_MAX don't make sense since the value passed means seconds. > Hence adding capability flag for this change isn't worth it. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140958 > > Signed-off-by: Luyao Huang > Signed-off-by: Martin Kletzander > --- > docs/formatdomain.html.in | 2 ++ > src/conf/domain_conf.c| 9 +++-- > src/conf/domain_conf.h| 2 +- > src/qemu/qemu_process.c | 2 +- > 4 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 40e2b29..7a11cc7 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -5630,6 +5630,8 @@ qemu-kvm -net nic,model=? /dev/null >only be made to the active guest. >If the QEMU driver is not at the right >revision, the attempt to set the period will fail. > + Large values might be ignored, but this only affects > + non-sensical numbers (i.e. many years). >Since 1.1.1, requires QEMU 1.5 > > Just a nitpick, I'd probably avoid word construction non-sensical in our docs (it's not even correct --> nonsensical) and simplify this to "Large values (i.e. many years) might be ignored." > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index e010040..b3d 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -10432,6 +10432,7 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, > char *model; > virDomainMemballoonDefPtr def; > xmlNodePtr save = ctxt->node; > +unsigned int period = 0; > > if (VIR_ALLOC(def) < 0) > return NULL; > @@ -10450,12 +10451,16 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, > } > > ctxt->node = node; > -if (virXPathUInt("string(./stats/@period)", ctxt, &def->period) < -1) { > +if (virXPathUInt("string(./stats/@period)", ctxt, &period) < -1) { > virReportError(VIR_ERR_XML_ERROR, "%s", > _("invalid statistics collection period")); > goto error; > } > > +def->period = period; > +if (def->period < 0) > +def->period = 0; > + > if (def->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) > VIR_DEBUG("Ignoring device address for none model Memballoon"); > else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) > @@ -18823,7 +18828,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, > virBufferAdjustIndent(&childrenBuf, indent + 2); > > if (def->period) > -virBufferAsprintf(&childrenBuf, "\n", > def->period); > +virBufferAsprintf(&childrenBuf, "\n", > def->period); > > if (virDomainDeviceInfoNeedsFormat(&def->info, flags) && > virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) { > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index ea463cb..ee0f5fd 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1556,7 +1556,7 @@ enum { > struct _virDomainMemballoonDef { > int model; > virDomainDeviceInfo info; > -unsigned int period; /* seconds between collections */ > +int period; /* seconds between collections */ > }; > > struct _virDomainNVRAMDef { > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index d1f089d..0f357d5 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -4390,7 +4390,7 @@ int qemuProcessStart(virConnectPtr conn, > virCommandPtr cmd = NULL; > struct qemuProcessHookData hookData; > unsigned long cur_balloon; > -unsigned int period = 0; > +int period = 0; > size_t i; > bool rawio_set = false; > char *nodeset = NULL; > -- > 2.3.2 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > ACK with the nitpick fixed. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] qemu: Don't duplicate errors when settings stats period
On 03/13/2015 05:17 PM, Martin Kletzander wrote: > In order not to leave old error messages set, this patch refactors the > code so the error is reported only when acted upon. The only such place > already rewrites any error, so cleaning up all the error reporting in > qemuMonitorSetMemoryStatsPeriod() is enough. > > +/** > + * qemuMonitorSetMemoryStatsPeriod: > + * > + * This function sets balloon stats update period. > + * > + * Returns 0 on success and -1 on error, but does *not* set an error. > + */ > int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, > int period) > { > int ret = -1; > VIR_DEBUG("mon=%p period=%d", mon, period); > > -if (!mon) { > -virReportError(VIR_ERR_INVALID_ARG, "%s", > - _("monitor must not be NULL")); > +if (!mon) > return -1; > -} > > -if (!mon->json) { > -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", > - _("JSON monitor is required")); > +if (!mon->json) > +return -1; > + > +if (period < 0) > return -1; > -} Hmm. It is a nice idea, but I guess you might have forgotten to check the actual return value in qemuProcessStart (there are actually 2 appearances in qemu_process.c) like we do in most cases. I found a piece of code (see below) where we check this correctly (so it's great you refactored this, otherwise reporting 2 identical messages would be sort of confusing) src/qemu/qemu_driver.c r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; if (r < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unable to set balloon driver collection period")); goto endjob; > if (qemuMonitorFindBalloonObjectPath(mon, "/") == 0) { > ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath, >period); > + > +/* > + * Most of the calls to this function are supposed to be > + * non-fatal and the only one that should be fatal wants its > + * own error message. More details for debugging will be in > + * the log file. > + */ > +if (ret < 0) > +virResetLastError(); > } > mon->ballooninit = true; > return ret; Everything else looks fine to me, though I think that other monitor calls should meet a similar fate sometime in the future. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] conf: Reorder elements inside memballoon
On 03/13/2015 05:17 PM, Martin Kletzander wrote: > All the devices we have format their address as its last sub-element, so > let's change memballoon to follow suit. Also adjust RNG to allow any > order of them so 'virsh edit' doesn't shout at us. > > Signed-off-by: Martin Kletzander > --- > docs/schemas/domaincommon.rng | 28 ++-- > src/conf/domain_conf.c | 30 > ++ > .../qemuxml2xmlout-balloon-device-period.xml | 30 > ++ > tests/qemuxml2xmltest.c| 1 + > 4 files changed, 60 insertions(+), 29 deletions(-) > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index b1d883f..b9d430a 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -3438,19 +3438,21 @@ >none > > > - > - > - > - > - > - > - > - > - > - > - > - > - > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index ae8688e..e010040 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -18810,7 +18810,8 @@ virDomainMemballoonDefFormat(virBufferPtr buf, > unsigned int flags) > { > const char *model = virDomainMemballoonModelTypeToString(def->model); > -bool noopts = true; > +virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; > +int indent = virBufferGetIndent(buf, false); > > if (!model) { > virReportError(VIR_ERR_INTERNAL_ERROR, > @@ -18819,27 +18820,24 @@ virDomainMemballoonDefFormat(virBufferPtr buf, > } > > virBufferAsprintf(buf, " -virBufferAdjustIndent(buf, 2); > +virBufferAdjustIndent(&childrenBuf, indent + 2); > > -if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) { > -virBufferAddLit(buf, ">\n"); > -if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) > -return -1; > -noopts = false; > -} > +if (def->period) > +virBufferAsprintf(&childrenBuf, "\n", > def->period); > > -if (def->period) { > -if (noopts) > -virBufferAddLit(buf, ">\n"); > -virBufferAsprintf(buf, "\n", def->period); > -noopts = false; > +if (virDomainDeviceInfoNeedsFormat(&def->info, flags) && > +virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) { > +virBufferFreeAndReset(&childrenBuf); > +return -1; > } > > -virBufferAdjustIndent(buf, -2); > -if (noopts) > +if (!virBufferUse(&childrenBuf)) { > virBufferAddLit(buf, "/>\n"); > -else > +} else { > +virBufferAddLit(buf, ">\n"); > +virBufferAddBuffer(buf, &childrenBuf); > virBufferAddLit(buf, "\n"); > +} > > return 0; > } > diff --git > a/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml > b/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml > new file mode 100644 > index 000..79e465a > --- /dev/null > +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml > @@ -0,0 +1,30 @@ > + > + QEMUGuest1 > + c7a5fdbd-edaf-9455-926a-d65c16db1809 > + 219136 > + 219136 > + 1 > + > +hvm > + > + > + > + destroy > + restart > + destroy > + > +/usr/bin/qemu > + > + > + > + > + > + > + > + > + > + > + function='0x0'/> > + > + > + > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index 8e12e84..9e4b3a2 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -354,6 +354,7 @@ mymain(void) > > /* These tests generate different XML */ > DO_TEST_DIFFERENT("balloon-device-auto"); > +DO_TEST_DIFFERENT("balloon-device-period"); > DO_TEST_DIFFERENT("channel-virtio-auto"); > DO_TEST_DIFFERENT("console-compat-auto"); > DO_TEST_DIFFERENT("disk-scsi-device-auto"); > -- > 2.3.2 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Looks good, ACK. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] util: Make sure the comment about virBufferAddBuffer is true
On 03/13/2015 05:17 PM, Martin Kletzander wrote: > Change it so it really *always* eats the @toadd buffer. > > Signed-off-by: Martin Kletzander > --- > src/util/virbuffer.c | 14 +- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c > index 96a0f16..0089d1b 100644 > --- a/src/util/virbuffer.c > +++ b/src/util/virbuffer.c > @@ -1,7 +1,7 @@ > /* > * virbuffer.c: buffers for libvirt > * > - * Copyright (C) 2005-2008, 2010-2014 Red Hat, Inc. > + * Copyright (C) 2005-2008, 2010-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 > @@ -188,23 +188,27 @@ virBufferAddBuffer(virBufferPtr buf, virBufferPtr toadd) > { > unsigned int needSize; > > -if (!buf || !toadd) > +if (!toadd) > return; > > +if (!buf) > +goto done; > + > if (buf->error || toadd->error) { > if (!buf->error) > buf->error = toadd->error; > -virBufferFreeAndReset(toadd); > -return; > +goto done; > } > > needSize = buf->use + toadd->use; > if (virBufferGrow(buf, needSize - buf->use) < 0) > -return; > +goto done; > > memcpy(&buf->content[buf->use], toadd->content, toadd->use); > buf->use += toadd->use; > buf->content[buf->use] = '\0'; > + > + done: > virBufferFreeAndReset(toadd); > } > > -- > 2.3.2 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > ACK, Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/12] Convert virDomainVcpuPinFindByVcpu into virDomainPinFindByVcpu
On Fri, Mar 13, 2015 at 11:11:52PM -0400, John Ferlan wrote: > Make common between Vcpu and IOThreads I feel like this sentence is missing something. > > Signed-off-by: John Ferlan > --- > src/conf/domain_conf.c | 24 > src/conf/domain_conf.h | 6 +++--- > src/libvirt_private.syms | 2 +- > src/qemu/qemu_driver.c | 12 ++-- > src/qemu/qemu_process.c | 12 ++-- > 5 files changed, 28 insertions(+), 28 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index efc01bd..75d75bc 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -16693,17 +16693,17 @@ virDomainPinIsDuplicate(virDomainPinDefPtr *def, > } > > virDomainPinDefPtr > -virDomainVcpuPinFindByVcpu(virDomainPinDefPtr *def, > - int nvcpupin, > - int vcpu) > +virDomainPinFindByVcpu(virDomainPinDefPtr *def, > + int npin, > + int id) The second reference to Vcpu should be removed as well: virDomainPinFindById or even simpler: virDomainPinFind since I can't imagine doing a find according to any other attribute. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/12] More cleanup from IOThreads changes
On Fri, Mar 13, 2015 at 11:11:44PM -0400, John Ferlan wrote: > During the review process a few things were pointed at as perhaps > needing some adjustments based on what was done for IOThreads. > Specifically a memory leak in PinVcpuFlags since PinIOThreads was > just a copy of the Vcpu code and secondarily since the IOThreads > code "reused" the virDomainVcpuPin* data structures and API's, those > should change names to be more generic. > > > John Ferlan (12): > qemu: Fix possible memory leak in qemuDomainPinVcpuFlags > Convert virDomainVcpuPinDefPtr to virDomainPinDefPtr > Convert virDomainPinDefPtr->vcpuid to virDomainPinDefPtr->id > Convert virDomainVcpuPinDefFree to virDomainPinDefFree > Convert virDomainVcpuPinDefArrayFree to virDomainPinDefArrayFree > Convert virDomainVcpuPinDefCopy into virDomainPinDefCopy > Convert virDomainVcpuPinIsDuplicate into virDomainPinIsDuplicate > Convert virDomainVcpuPinFindByVcpu into virDomainPinFindByVcpu > Replace virDomainVcpuPinAdd with virDomainPinAdd > Replace virDomainIOThreadsPinAdd with virDomainPinAdd > Replace virDomainVcpuPinDel with virDomainPinDel > Remove virDomainIOThreadsPinDel > > src/conf/domain_conf.c | 234 > +-- > src/conf/domain_conf.h | 58 +--- > src/libvirt_private.syms | 16 ++-- > src/libxl/libxl_domain.c | 2 +- > src/libxl/libxl_driver.c | 18 ++-- > src/qemu/qemu_cgroup.c | 12 +-- > src/qemu/qemu_cgroup.h | 4 +- > src/qemu/qemu_driver.c | 104 +++-- > src/qemu/qemu_process.c | 16 ++-- > src/xen/xend_internal.c | 10 +- > 10 files changed, 204 insertions(+), 270 deletions(-) > ACK series. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v10 4/4] domifaddr: Add virsh support
On Mon, Mar 16, 2015 at 08:43:32AM -0400, John Ferlan wrote: > > > On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > > From: Nehal J Wani > > > > tools/virsh-domain-monitor.c > >* Introduce new command : domifaddr > > Usage: domifaddr [interface] [--full] [--source lease|agent] > > > > Example outputs: > > virsh # domifaddr f20 > > Name MAC address Protocol Address > > > > --- > > lo 00:00:00:00:00:00ipv4 127.0.0.1/8 > > - -ipv6 ::1/128 > > eth0 52:54:00:2e:45:ceipv4 10.1.33.188/24 > > - -ipv6 2001:db8:0:f101::2/64 > > - -ipv6 fe80::5054:ff:fe2e:45ce/64 > > eth1 52:54:00:b1:70:19ipv4 192.168.105.201/16 > > - -ipv4 192.168.201.195/16 > > - -ipv6 fe80::5054:ff:feb1:7019/64 > > eth2 52:54:00:36:2a:e5N/A N/A > > eth3 52:54:00:20:70:3dipv4 192.168.105.240/16 > > - -ipv6 fe80::5054:ff:fe20:703d/64 > > > > Does the above list now change with the default --source of lease? Yeah, it should be using vnet0, vnet1, etc instead of eth0, eth1, etc at the very least. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpm-build: use pkg-config to detect wireshark presence
On Mon, Mar 16, 2015 at 01:41:15PM +0100, Pavel Hrdina wrote: > Wireshark supports pkg-config since 1.11.3. Right now we build > wireshark-dissectior tool as default trough rpm build only on > fedora >= 21 and there is newer wireshark that supports pkg-config. > If someone wants to build libvirt with wireshark-dissector against older > wireshark, they should specify the location by hand. > > This patch is mainly to fix wrong dependency on wireshark binary as it > doesn't make sense to require that binary file to just get version info > of that package in makefile. > > Signed-off-by: Pavel Hrdina Be nice to go one step further and switch over to using LIBVIRT_CHECK_PKG and having the code in m4/virt-wireshark.m4. Just copy virt-fuse.m4 as a equivalently simple module Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v10 1/4] domifaddr: Implement the public APIs
On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > From: Nehal J Wani > > Define helper function virDomainInterfaceFree, which allows > the upper layer application to free the domain interface object > conveniently. > > The API is going to provide multiple methods by flags, e.g. > * Query guest agent > * Parse DHCP lease file > > include/libvirt/libvirt-domain.h > * Define virDomainInterfaceAddresses, virDomainInterfaceFree > * Define structs virDomainInterface, virDomainIPAddress > > src/driver-hypervisor.h: > * Define domainInterfaceAddresses > > src/libvirt-domain.c: > * Implement virDomainInterfaceAddresses > * Implement virDomainInterfaceFree > > src/libvirt_public.syms: > * Export the new symbols > > Signed-off-by: Nehal J Wani > --- > include/libvirt/libvirt-domain.h | 32 ++ > src/driver-hypervisor.h | 6 ++ > src/libvirt-domain.c | 123 > +++ > src/libvirt_public.syms | 2 + > 4 files changed, 163 insertions(+) > > diff --git a/include/libvirt/libvirt-domain.h > b/include/libvirt/libvirt-domain.h > index 9487b80..8cc7fe8 100644 > --- a/include/libvirt/libvirt-domain.h > +++ b/include/libvirt/libvirt-domain.h > @@ -3725,5 +3725,37 @@ typedef struct _virTypedParameter virMemoryParameter; > */ > typedef virMemoryParameter *virMemoryParameterPtr; > > +typedef enum { > +VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE = 0, /* Parse DHCP lease file */ > +VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT = 1, /* Query qemu guest agent > */ > + > +# ifdef VIR_ENUM_SENTINELS > +VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LAST > +# endif > +} virDomainInterfaceAddressesSource; > + > +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress; > +typedef virDomainIPAddress *virDomainIPAddressPtr; > +struct _virDomainInterfaceIPAddress { > +int type;/* virIPAddrType */ > +char *addr; /* IP address */ > +unsigned int prefix; /* IP address prefix */ > +}; > + > +typedef struct _virDomainInterface virDomainInterface; > +typedef virDomainInterface *virDomainInterfacePtr; > +struct _virDomainInterface { > +char *name; /* interface name */ > +char *hwaddr; /* hardware address */ > +unsigned int naddrs;/* number of items in @addrs */ > +virDomainIPAddressPtr addrs;/* array of IP addresses */ > +}; > + > +int virDomainInterfaceAddresses(virDomainPtr dom, > +virDomainInterfacePtr **ifaces, > +unsigned int source, > +unsigned int flags); > + > +void virDomainInterfaceFree(virDomainInterfacePtr iface); > > #endif /* __VIR_LIBVIRT_DOMAIN_H__ */ > diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h > index c2b4cd8..0e729c4 100644 > --- a/src/driver-hypervisor.h > +++ b/src/driver-hypervisor.h > @@ -1178,6 +1178,11 @@ typedef int > unsigned int cellCount, > unsigned int flags); > > +typedef int > +(*virDrvDomainInterfaceAddresses)(virDomainPtr dom, > + virDomainInterfacePtr **ifaces, > + unsigned int source, > + unsigned int flags); > > typedef struct _virHypervisorDriver virHypervisorDriver; > typedef virHypervisorDriver *virHypervisorDriverPtr; > @@ -1404,6 +1409,7 @@ struct _virHypervisorDriver { > virDrvConnectGetAllDomainStats connectGetAllDomainStats; > virDrvNodeAllocPages nodeAllocPages; > virDrvDomainGetFSInfo domainGetFSInfo; > +virDrvDomainInterfaceAddresses domainInterfaceAddresses; > }; > > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index 04545fd..cf36d30 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c > @@ -11338,3 +11338,126 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) > VIR_FREE(info->devAlias[i]); > VIR_FREE(info->devAlias); > } > + > +/** > + * virDomainInterfaceAddresses: > + * @dom: domain object > + * @ifaces: pointer to an array of pointers pointing to interface objects > + * @source: one of the virDomainInterfaceAddressesSource constants > + * @flags: currently unused, pass zero > + * > + * Return a pointer to the allocated array of pointers to interfaces > + * present in given domain along with their IP and MAC addresses. Note that > + * single interface can have multiple or even 0 IP addresses. > + * > + * This API dynamically allocates the virDomainInterfacePtr struct based on > + * how many interfaces domain @dom has, usually there's 1:1 correlation. The > + * count of the interfaces is returned as the return value. > + * > + * If @source is VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE, the DHCP lease > + * file associated with any virtual networks will be examined to obtain > + * the interface addresses. This only returns data for interfaces whic
Re: [libvirt] [PATCH v10 2/4] domifaddr: Implement the remote protocol
On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > From: Nehal J Wani > > daemon/remote.c >* Define remoteSerializeDomainInterface, > remoteDispatchDomainInterfaceAddresses > > src/remote/remote_driver.c >* Define remoteDomainInterfaceAddresses > > src/remote/remote_protocol.x >* New RPC procedure: REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES >* Define structs remote_domain_ip_addr, remote_domain_interface, > remote_domain_interfaces_addresse_args, > remote_domain_interface_addresses_ret >* Introduce upper bounds (to handle DoS attacks): > REMOTE_DOMAIN_INTERFACE_MAX = 2048 > REMOTE_DOMAIN_IP_ADDR_MAX = 2048 > Restrictions on the maximum number of aliases per interface were > removed after kernel v2.0, and theoretically, at present, there > are no upper limits on number of interfaces per virtual machine > and on the number of IP addresses per interface. > > src/remote_protocol-structs >* New structs added > > Signed-off-by: Nehal J Wani > --- > daemon/remote.c | 124 > +++ > src/remote/remote_driver.c | 103 +++ > src/remote/remote_protocol.x | 37 - > src/remote_protocol-structs | 25 + > 4 files changed, 288 insertions(+), 1 deletion(-) > > diff --git a/daemon/remote.c b/daemon/remote.c > index ff64eeb..f2b970a 100644 > --- a/daemon/remote.c > +++ b/daemon/remote.c > @@ -6510,6 +6510,130 @@ remoteDispatchDomainGetFSInfo(virNetServerPtr server > ATTRIBUTE_UNUSED, > } > > > +static int > +remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces, > + unsigned int ifaces_count, > + remote_domain_interface_addresses_ret *ret) > +{ > +size_t i, j; > + > +if (ifaces_count > REMOTE_DOMAIN_INTERFACE_MAX) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Number of interfaces, %d exceeds the max limit: > %d"), > + ifaces_count, REMOTE_DOMAIN_INTERFACE_MAX); > +return -1; > +} > + > +if (VIR_ALLOC_N(ret->ifaces.ifaces_val, ifaces_count) < 0) > +return -1; > + > +ret->ifaces.ifaces_len = ifaces_count; > + > +for (i = 0; i < ifaces_count; i++) { > +virDomainInterfacePtr iface = ifaces[i]; > +remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]); > + > +if ((VIR_STRDUP(iface_ret->name, iface->name)) < 0) > +goto cleanup; > + > +if ((VIR_STRDUP(iface_ret->hwaddr, iface->hwaddr)) < 0) > +goto cleanup; > + > +if (iface->naddrs > REMOTE_DOMAIN_IP_ADDR_MAX) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Number of interfaces, %d exceeds the max > limit: %d"), > + iface->naddrs, REMOTE_DOMAIN_IP_ADDR_MAX); > +goto cleanup; > +} > + > +if (VIR_ALLOC_N(iface_ret->addrs.addrs_val, > +iface->naddrs) < 0) > +goto cleanup; > + > +iface_ret->addrs.addrs_len = iface->naddrs; > + > +for (j = 0; j < iface->naddrs; j++) { > +virDomainIPAddressPtr ip_addr = &(iface->addrs[j]); > +remote_domain_ip_addr *ip_addr_ret = > +&(iface_ret->addrs.addrs_val[j]); > + > +if (VIR_STRDUP(ip_addr_ret->addr, ip_addr->addr) < 0) > +goto cleanup; > + > +ip_addr_ret->prefix = ip_addr->prefix; > +ip_addr_ret->type = ip_addr->type; > +} > +} > + > +return 0; > + > + cleanup: > +if (ret->ifaces.ifaces_val) { > +for (i = 0; i < ifaces_count; i++) { > +remote_domain_interface *iface_ret = > &(ret->ifaces.ifaces_val[i]); > +VIR_FREE(iface_ret->name); > +VIR_FREE(iface_ret->hwaddr); > +for (j = 0; j < iface_ret->addrs.addrs_len; j++) { > +remote_domain_ip_addr *ip_addr = > +&(iface_ret->addrs.addrs_val[j]); > +VIR_FREE(ip_addr->addr); > +} > +} > +VIR_FREE(ret->ifaces.ifaces_val); > +} > + > +return -1; > +} > + > + > +static int > +remoteDispatchDomainInterfaceAddresses(virNetServerPtr server > ATTRIBUTE_UNUSED, > + virNetServerClientPtr client, > + virNetMessagePtr msg ATTRIBUTE_UNUSED, > + virNetMessageErrorPtr rerr, > + > remote_domain_interface_addresses_args *args, > + remote_domain_interface_addresses_ret > *ret) > +{ > +size_t i; > +int rv = -1; > +virDomainPtr dom = NULL; > +virDomainInterfacePtr *ifaces = NULL; > +int ifaces_count = 0; > +struct daemonClientPrivate *priv = > +virNetServerClientGetPrivateData(client); > + > +
[libvirt] [PATCH] rpm-build: use pkg-config to detect wireshark presence
Wireshark supports pkg-config since 1.11.3. Right now we build wireshark-dissectior tool as default trough rpm build only on fedora >= 21 and there is newer wireshark that supports pkg-config. If someone wants to build libvirt with wireshark-dissector against older wireshark, they should specify the location by hand. This patch is mainly to fix wrong dependency on wireshark binary as it doesn't make sense to require that binary file to just get version info of that package in makefile. Signed-off-by: Pavel Hrdina --- configure.ac | 26 ++ 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/configure.ac b/configure.ac index 2fedd1a..75a0688 100644 --- a/configure.ac +++ b/configure.ac @@ -2669,25 +2669,12 @@ if test "$with_wireshark_dissector" != "no"; then ]) dnl Search for wireshark(or tshark) command -AC_PATH_PROG([WIRESHARK], [wireshark]) -AC_PATH_PROG([WIRESHARK], [tshark]) -if test -z "$WIRESHARK"; then -LIBVIRT_WS_HANDLE_ERROR([command not found wireshark or tshark]) -else -dnl Check for wireshark headers -save_CPPFLAGS="$CPPFLAGS" -WS_DISSECTOR_CPPFLAGS="$WS_DISSECTOR_CPPFLAGS -I`dirname $WIRESHARK`/../include/wireshark" -CPPFLAGS="$CPPFLAGS $WS_DISSECTOR_CPPFLAGS" -AC_CHECK_HEADERS([wireshark/config.h],, [ -LIBVIRT_WS_HANDLE_ERROR([wireshark/config.h is required for wireshark-dissector support]) -]) -AC_CHECK_HEADERS([wireshark/epan/packet.h wireshark/epan/dissectors/packet-tcp.h],, [ -LIBVIRT_WS_HANDLE_ERROR([wireshark/epan/{packet,packet-tcp}.h are required for wireshark-dissector support]) -], [ - #include -]) -CPPFLAGS="$save_CPPFLAGS" -fi +PKG_CHECK_MODULES([WIRESHARK], [wireshark], [ + WS_DISSECTOR_CPPFLAGS="$WS_DISSECTOR_CPPFLAGS `$PKG_CONFIG --cflags wireshark`" + ws_version=`$PKG_CONFIG --modversion wireshark` +], [ + LIBVIRT_WS_HANDLE_ERROR([pkg-config 'wireshark' is required for wireshark-dissector support]) +]) if test "$with_wireshark_dissector" != "no"; then with_wireshark_dissector=yes fi @@ -2701,7 +2688,6 @@ AC_ARG_WITH([ws-plugindir], [ws_plugindir=$withval]) if test "$with_wireshark_dissector" != "no" && test -z "$ws_plugindir"; then -ws_version=`$WIRESHARK -v | head -1 | cut -f 2 -d' '` ws_plugindir="$libdir/wireshark/plugins/$ws_version" fi AC_SUBST([ws_plugindir]) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v10 4/4] domifaddr: Add virsh support
On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > From: Nehal J Wani > > tools/virsh-domain-monitor.c >* Introduce new command : domifaddr > Usage: domifaddr [interface] [--full] [--source lease|agent] > > Example outputs: > virsh # domifaddr f20 > Name MAC address Protocol Address > > --- > lo 00:00:00:00:00:00ipv4 127.0.0.1/8 > - -ipv6 ::1/128 > eth0 52:54:00:2e:45:ceipv4 10.1.33.188/24 > - -ipv6 2001:db8:0:f101::2/64 > - -ipv6 fe80::5054:ff:fe2e:45ce/64 > eth1 52:54:00:b1:70:19ipv4 192.168.105.201/16 > - -ipv4 192.168.201.195/16 > - -ipv6 fe80::5054:ff:feb1:7019/64 > eth2 52:54:00:36:2a:e5N/A N/A > eth3 52:54:00:20:70:3dipv4 192.168.105.240/16 > - -ipv6 fe80::5054:ff:fe20:703d/64 > Does the above list now change with the default --source of lease? Since the subsequent example lists --source agent, I suspect the above may not be "correct" for eth0. > virsh # domifaddr f20 eth1 --source lease > Name MAC address Protocol Address > > --- > eth1 52:54:00:b1:70:19ipv4 192.168.105.201/16 > - -ipv4 192.168.201.195/16 > - -ipv6 fe80::5054:ff:feb1:7019/64 > > virsh # domifaddr f20 eth0 --source agent --full > Name MAC address Protocol Address > > --- > eth0 52:54:00:2e:45:ceipv4 10.1.33.188/24 > eth0 52:54:00:2e:45:ceipv6 2001:db8:0:f101::2/64 > eth0 52:54:00:2e:45:ceipv6 fe80::5054:ff:fe2e:45ce/64 > > tools/virsh.pod >* Document new command > > Signed-off-by: Nehal J Wani > --- > tools/virsh-domain-monitor.c | 146 > +++ > tools/virsh.pod | 16 + > 2 files changed, 162 insertions(+) > > diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c > index 464ac11..92ce152 100644 > --- a/tools/virsh-domain-monitor.c > +++ b/tools/virsh-domain-monitor.c > @@ -2197,6 +2197,146 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) > return ret; > } > > +/* "domifaddr" command > + */ > +static const vshCmdInfo info_domifaddr[] = { > +{"help", N_("Get network interfaces' addresses for a running domain")}, > +{"desc", N_("Get network interfaces' addresses for a running domain")}, > +{NULL, NULL} > +}; > + > +static const vshCmdOptDef opts_domifaddr[] = { > +{.name = "domain", > + .type = VSH_OT_DATA, > + .flags = VSH_OFLAG_REQ, > + .help = N_("domain name, id or uuid")}, > +{.name = "interface", > + .type = VSH_OT_STRING, > + .flags = VSH_OFLAG_NONE, > + .help = N_("network interface name")}, > +{.name = "full", > + .type = VSH_OT_BOOL, > + .flags = VSH_OFLAG_NONE, > + .help = N_("display full fields")}, > +{.name = "source", > + .type = VSH_OT_STRING, > + .flags = VSH_OFLAG_NONE, > + .help = N_("address source: 'lease' or 'agent'")}, > +{.name = NULL} > +}; > + > +static bool > +cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) > +{ > +virDomainPtr dom = NULL; > +const char *interface = NULL; > +virDomainInterfacePtr *ifaces = NULL; > +size_t i, j; > +int ifaces_count = 0; > +bool ret = false; > +bool full = vshCommandOptBool(cmd, "full"); > +const char *sourcestr = NULL; > +int source = VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE; > + > +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) > +return false; > + > +if (vshCommandOptString(cmd, "interface", &interface) < 0) > +goto cleanup; > +if (vshCommandOptString(cmd, "source", &sourcestr) < 0) > +goto cleanup; > + > +if (sourcestr) { > +if (STREQ(sourcestr, "lease")) { > +source = VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE; > +} else if (STREQ(sourcestr, "agent")) { > +source = VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT; > +} else { > +vshError(ctl, _("Unknown data source '%s'"), sourcestr); > +goto cleanup; > +} > +} > + > +if ((ifaces_count = virDomainInterfaceAddresses(dom, &ifaces, source, > 0)) < 0) { > +vshError(ctl, _("Failed to query for interfaces addresses")); > +goto cleanup; > +} > + > +vshPrintExtra(ctl, " %-10s %-20s %-8s %
Re: [libvirt] AUTHORS.in: added myself into commiters list
On Mon, Mar 16, 2015 at 09:34:56 +0100, Erik Skultety wrote: The actual patch is missing :) Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v10 3/4] domifaddr: Implement the API for qemu
On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > From: Nehal J Wani > > By querying the qemu guest agent with the QMP command > "guest-network-get-interfaces" and converting the received JSON > output to structured objects. > > Although "ifconfig" is deprecated, IP aliases created by "ifconfig" > are supported by this API. The legacy syntax of an IP alias is: > ":". Since we want all aliases to be clubbed > under parent interface, simply stripping ":" suffices. > Note that IP aliases formed by "ip" aren't visible to "ifconfig", > and aliases created by "ip" do not have any specific name. But > we are lucky, as qemu guest agent detects aliases created by both. > > src/qemu/qemu_agent.h: > * Define qemuAgentGetInterfaces > > src/qemu/qemu_agent.c: > * Implement qemuAgentGetInterface > > src/qemu/qemu_driver.c: > * New function qemuGetDHCPInterfaces > * New function qemuDomainInterfaceAddresses > > src/remote_protocol-sructs: > * Define new structs > > tests/qemuagenttest.c: > * Add new test: testQemuAgentGetInterfaces > Test cases for IP aliases, 0 or multiple ipv4/ipv6 address(es) > > Signed-off-by: Nehal J Wani > --- > src/qemu/qemu_agent.c | 204 > + > src/qemu/qemu_agent.h | 4 + > src/qemu/qemu_driver.c | 175 ++ > tests/qemuagenttest.c | 188 + > 4 files changed, 571 insertions(+) > ACK - there's a NIT and a comment that follows, I'll let you decide how to handle. John > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c > index 5fcc40f..8155006 100644 > --- a/src/qemu/qemu_agent.c > +++ b/src/qemu/qemu_agent.c > @@ -1953,3 +1953,207 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, > virDomainFSInfoPtr **info, > virJSONValueFree(reply); > return ret; > } > + > +/* > + * qemuAgentGetInterfaces: > + * @mon: Agent monitor > + * @ifaces: pointer to an array of pointers pointing to interface objects > + * > + * Issue guest-network-get-interfaces to guest agent, which returns a > + * list of interfaces of a running domain along with their IP and MAC > + * addresses. > + * > + * Returns: number of interfaces on success, -1 on error. > + */ > +int > +qemuAgentGetInterfaces(qemuAgentPtr mon, > + virDomainInterfacePtr **ifaces) > +{ > +int ret = -1; > +size_t i, j; > +int size = -1; > +virJSONValuePtr cmd = NULL; > +virJSONValuePtr reply = NULL; > +virJSONValuePtr ret_array = NULL; > +size_t ifaces_count = 0; > +size_t addrs_count = 0; > +virDomainInterfacePtr *ifaces_ret = NULL; > +virHashTablePtr ifaces_store = NULL; > +char **ifname = NULL; > + > +/* Hash table to handle the interface alias */ > +if (!(ifaces_store = virHashCreate(ifaces_count, NULL))) { > +virHashFree(ifaces_store); > +return -1; > +} > + > +if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL))) > +goto cleanup; > + > +if (qemuAgentCommand(mon, cmd, &reply, false, > VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 || > +qemuAgentCheckError(cmd, reply) < 0) { > +goto cleanup; > +} > + > +if (!(ret_array = virJSONValueObjectGet(reply, "return"))) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("qemu agent didn't provide 'return' field")); > +goto cleanup; > +} > + > +if ((size = virJSONValueArraySize(ret_array)) < 0) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("qemu agent didn't return an array of interfaces")); > +goto cleanup; > +} > + > +for (i = 0; i < size; i++) { > +virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i); > +virJSONValuePtr ip_addr_arr = NULL; > +const char *hwaddr, *ifname_s, *name = NULL; > +int ip_addr_arr_size; > +virDomainInterfacePtr iface = NULL; > + > +/* Shouldn't happen but doesn't hurt to check neither */ > +if (!tmp_iface) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("something has went really wrong")); > +goto error; > +} > + > +/* interface name is required to be presented */ > +name = virJSONValueObjectGetString(tmp_iface, "name"); > +if (!name) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("qemu agent didn't provide 'name' field")); > +goto error; > +} > + > +/* Handle interface alias (:) */ > +ifname = virStringSplit(name, ":", 2); > +ifname_s = ifname[0]; > + > +iface = virHashLookup(ifaces_store, ifname_s); > + > +/* If the hash table doesn't contain this iface, add it */ > +if (!iface) { > +if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0) > +goto error; > + > +
Re: [libvirt] [libvirt-perl PATCHv2 1/2] Add virDomainGetIOThreads and virDomainPinIOThread bindings
On Tue, Mar 10, 2015 at 11:04:00AM -0400, John Ferlan wrote: > Test results in the following output: > > $ perl examples/iothreadsinfo.pl > Addr > VMM type: QEMU > ... > Domain: { > ID: 2 'f18iothr' > UUID: fb9f7826-b5d7-4f74-b962-7181ef3fc9ec > IOThread: { > affinity: 0010 > number: 1 > } > IOThread: { > affinity: 0001 > number: 2 > } > IOThread: { > affinity: 1100 > number: 3 > } > } > > Signed-off-by: John Ferlan > --- > Changes | 2 +- > Virt.xs | 42 ++ > examples/iothreadsinfo.pl | 33 + > lib/Sys/Virt/Domain.pm| 17 + > 4 files changed, 93 insertions(+), 1 deletion(-) > create mode 100644 examples/iothreadsinfo.pl > > diff --git a/Changes b/Changes > index b62ee24..88f648c 100644 > --- a/Changes > +++ b/Changes > @@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt > > 1.2.14 2015-00-00 > > - - XXX > + - Add virDomainGetIOThreads and virDomainPinIOThread API bindings > > 1.2.13 2015-03-05 > > diff --git a/Virt.xs b/Virt.xs > index f9ec7a4..56e143d 100644 > --- a/Virt.xs > +++ b/Virt.xs > @@ -5014,6 +5014,48 @@ get_emulator_pin_info(dom, flags=0) >RETVAL > > > +void > +get_iothread_info(dom, flags=0) > + virDomainPtr dom; > + unsigned int flags; > + PREINIT: > + virDomainIOThreadInfoPtr *iothrinfo; > + int niothreads; > + int i; > + PPCODE: > + if ((niothreads = virDomainGetIOThreadsInfo(dom, &iothrinfo, > + flags)) < 0) > + _croak_error(); > + > + EXTEND(SP, niothreads); > + for (i = 0 ; i < niothreads ; i++) { > + HV *rec = newHV(); > + (void)hv_store(rec, "number", 6, > + newSViv(iothrinfo[i]->iothread_id), 0); > + (void)hv_store(rec, "affinity", 8, > + newSVpvn((char*)iothrinfo[i]->cpumap, > + iothrinfo[i]->cpumaplen), 0); > + PUSHs(newRV_noinc((SV *)rec)); > + } > + > + Safefree(iothrinfo); Opps, we should have been calling virDomainIOThreadsInfoFree on each element of iothrinfo, and then using free() rather than Safefree(), since the memory was allocated by Libvirt rather than by Perl. > +void > +pin_iothread(dom, iothread_id, mask, flags=0) > + virDomainPtr dom; > + unsigned int iothread_id; > + SV *mask; > + unsigned int flags; > +PREINIT: > + STRLEN masklen; > + unsigned char *maps; > + PPCODE: > + maps = (unsigned char *)SvPV(mask, masklen); > + if (virDomainPinVcpuFlags(dom, iothread_id, maps, masklen, flags) < 0) > + _croak_error(); And s/VcpuFlags/IOThreads/ here. BTW, I noticed these mistakes by running the API coverage test suite make test TEST_MAINTAINER=1 which reports anything in libvirt*.h that is not used from the Perl code. I've pushed the obvious fixes. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] PowerPC : Do not allow an empty model spec for 'host-model'
[PATCH] PowerPC : Do not allow an empty model spec for 'host-model' On PowerPC, a guest VM having CPU mode as 'host-model' represents a 'compat' mode VM. This cannot have a NULL CPU model. This commit forbids such a guest definition. Signed-off-by: Prerna Saxena --- src/cpu/cpu_powerpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index c77374c..62ad92b 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -549,12 +549,12 @@ ppcUpdate(virCPUDefPtr guest, const virCPUDef *host) { switch ((virCPUMode) guest->mode) { -case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_HOST_PASSTHROUGH: guest->match = VIR_CPU_MATCH_EXACT; virCPUDefFreeModel(guest); return virCPUDefCopyModel(guest, host, true); +case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_CUSTOM: return 0; -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: fix broken block job handling
On Fri, Mar 13, 2015 at 11:24:25 -0600, Eric Blake wrote: > On 03/13/2015 10:25 AM, Peter Krempa wrote: > > Block job handling violates our usage of domain jobs and changes disk source > > definition behind our back. > > > > Peter Krempa (3): > > qemu: process: Export qemuProcessFindDomainDiskByAlias > > qemu: event: Don't fiddle with disk backing trees without a job > > qemu: Disallow concurrent block jobs on a single disk > > On description alone, this should help solve the crash that Shanzhi > observed: > https://www.redhat.com/archives/libvir-list/2015-March/msg00656.html > I've pushed this series. While my suggestion to check also the length of the backing chain reported by qemu would fix that crash as a symptom, this fixes the reason why it happened. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX
On Fri, Mar 13, 2015 at 04:09:57PM -0300, Eduardo Habkost wrote: > With the Intel microcode update that removed HLE and RTM, there will be > different kinds of Haswell and Broadwell CPUs out there: some that still > have the HLE and RTM features, and some that don't have the HLE and RTM > features. On both cases people may be willing to use the pc-*-2.3 > machine-types. > > So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU > models, for hosts that have Haswell and Broadwell CPUs without TSX support. > > Signed-off-by: Eduardo Habkost The addition of Haswell-noTSX looks good to me. I'm unclear on whether we truely need Broadwell-noTSX though. Did Intel actually ship any Broadwell production silicon in which the microcode disables this feature, or was it only a problem on pre-production samples of Broadwell ? If the latter, I'd say we don't need to have a Broadwell-noTSX model added. Perhaps Jun/Don can confirm from Intel's side. > --- > target-i386/cpu.c | 69 > +++ > 1 file changed, 69 insertions(+) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index de3cdce..b693bab 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1073,6 +1073,39 @@ static X86CPUDefinition builtin_x86_defs[] = { > .model_id = "Intel Xeon E3-12xx v2 (Ivy Bridge)", > }, > { > +.name = "Haswell-noTSX", > +.level = 0xd, > +.vendor = CPUID_VENDOR_INTEL, > +.family = 6, > +.model = 60, > +.stepping = 1, > +.features[FEAT_1_EDX] = > +CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX | > +CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA > | > +CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 | > +CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE | > +CPUID_DE | CPUID_FP87, > +.features[FEAT_1_ECX] = > +CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES | > +CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 | > +CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | > +CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 | > +CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE | > +CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND, > +.features[FEAT_8000_0001_EDX] = > +CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX | > +CPUID_EXT2_SYSCALL, > +.features[FEAT_8000_0001_ECX] = > +CPUID_EXT3_LAHF_LM, > +.features[FEAT_7_0_EBX] = > +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | > +CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | > +CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID, > +.features[FEAT_XSAVE] = > +CPUID_XSAVE_XSAVEOPT, > +.xlevel = 0x800A, > +.model_id = "Intel Core Processor (Haswell, no TSX)", > +},{ > .name = "Haswell", > .level = 0xd, > .vendor = CPUID_VENDOR_INTEL, > @@ -1108,6 +1141,42 @@ static X86CPUDefinition builtin_x86_defs[] = { > .model_id = "Intel Core Processor (Haswell)", > }, > { > +.name = "Broadwell-noTSX", > +.level = 0xd, > +.vendor = CPUID_VENDOR_INTEL, > +.family = 6, > +.model = 61, > +.stepping = 2, > +.features[FEAT_1_EDX] = > +CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX | > +CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA > | > +CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 | > +CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE | > +CPUID_DE | CPUID_FP87, > +.features[FEAT_1_ECX] = > +CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES | > +CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 | > +CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | > +CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 | > +CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE | > +CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND, > +.features[FEAT_8000_0001_EDX] = > +CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX | > +CPUID_EXT2_SYSCALL, > +.features[FEAT_8000_0001_ECX] = > +CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH, > +.features[FEAT_7_0_EBX] = > +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | > +CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | > +CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID | > +CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | > +CPUID_7_0_EBX_SMAP, > +.features[FEAT_XSAVE] = > +CPUID_XSAVE_XSAVEOPT, > +.xlevel = 0x800A, > +.model_id = "Intel
Re: [libvirt] [PATCH 3/3] qemu: Disallow concurrent block jobs on a single disk
On Fri, Mar 13, 2015 at 13:36:13 -0600, Eric Blake wrote: > On 03/13/2015 10:25 AM, Peter Krempa wrote: > > While qemu may be prepared to do this libvirt is not. Forbid the block > > ops until we fix our code. > > --- > > src/conf/domain_conf.h | 1 + > > src/qemu/qemu_domain.c | 23 +++ > > src/qemu/qemu_domain.h | 2 ++ > > src/qemu/qemu_driver.c | 28 +--- > > 4 files changed, 39 insertions(+), 15 deletions(-) > > > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > > index ea463cb..6f2df46 100644 > > --- a/src/conf/domain_conf.h > > +++ b/src/conf/domain_conf.h > > @@ -664,6 +664,7 @@ struct _virDomainDiskDef { > > int tray_status; /* enum virDomainDiskTray */ > > int removable; /* enum virTristateSwitch */ > > > > +bool blockjob; > > Might be worth a FIXME comment acknowledging that a bool will be > insufficient when we update our code to support parallel jobs. Ok, I'll add a note. > > > + > > +bool > > +qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk) > > +{ > > +if (disk->mirror) { > > +virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, > > + _("disk '%s' already in active block job"), > > + disk->dst); > > + > > +return true; > > +} > > + > > +if (disk->blockjob) { > > +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, > > + _("disk '%s' already in active block job"), > > + disk->dst); > > +return true; > > +} > > Shorter as 'if (disk->mirror || disk->blockjob)', up to you if you want > to compress the common code. The errors have different error codes and I wanted to avoid a ternary operator. > > ACK. Looks like you have correctly locked out blockpull, blockcopy, and > blockcommit as the three jobs that are all mutually exclusive according > to our current abilities. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Revert "target-i386: Disable HLE and RTM on Haswell & Broadwell"
On Fri, Mar 13, 2015 at 04:09:56PM -0300, Eduardo Habkost wrote: > This reverts commit 13704e4c455770d500d6b87b117e32f0d01252c9. > > With the Intel microcode update that removed HLE and RTM, there will be > different kinds of Haswell and Broadwell CPUs out there: some that still > have the HLE and RTM features, and some that don't have the HLE and RTM > features. On both cases people may be willing to use the pc-*-2.3 > machine-types. > > So instead of making the CPU model results confusing by making it depend > on the machine-type, keep HLE and RTM on the existing Haswell and > Broadwell CPU models. The plan is to introduce "Haswell-noTSX" and > "Broadwell-noTSX" CPU models later, for people who have CPUs that don't > have TSX feature available. > > Signed-off-by: Eduardo Habkost Yep, in this situation we need to support both "models" of CPU, so changing based on machine type is inappropriate in this scenario. Reviewed-by: Daniel P. Berrange Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Check for negative port values in network drive configuration
On 02/20/2015 06:09 PM, Peter Krempa wrote: > I think Erik's patch is actually fine in the context of the code. > > Peter OK, now pushed, thanks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] AUTHORS.in: added myself into commiters list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list