Re: [libvirt] [PATCH] storage: fix memory leak with encrypted images
Eric Blake wrote: > Jim Fehlig reported a regression found by libvirt-TCK tests: > > >> ~ # perl /usr/share/libvirt-tck/tests/qemu/100-disk-encryption.t >> > ... > >> ok 4 - defined persistent domain config >> # Starting inactive domain config >> libvirt error code: 1, message: internal error: unable to execute QEMU >> command >> 'cont': 'drive-ide0-0-1' >> (/var/cache/libvirt-tck/300-disk-encryption/demo.qcow2) is encrypted >> > > Commit 2279d560 converted a boolean into a pointer with the intent of > transferring that pointer out of a temporary object into the caller's > data structure. The temporary structure meant that meta->encryption > was always NULL on entry, so we could get away with blindly allocating > the pointer when the header said so. But later commits then tweaked > things to do backing chain detection in-place, rather than via a > temporary object; this has the net result that meta->encryption can be > non-NULL on entry. For reference, bisected and found the 'later commit' you mentioned to be commit 8823272d41a259c1246c05d89f40ad3614fba58c Author: Peter Krempa Date: Fri Apr 18 14:49:54 2014 +0200 util: storage: Invert the way recursive metadata retrieval works > Not only did this turn the latent behavior into a > memory leak, it is also a behavior regression: blindly allocating a > new pointer wipes out what secrets we already knew about the chain, > making it impossible to restart the domain. > > Of course, no one in their right mind should be relying on qcow2 > encryption - it is fundamentally flawed. And sadly, the TCK tests > don't get run often enough, and this shows that our virstoragetest > does not exercise encrypted images at all. Otherwise, we could > have avoided a release containing this regression. > > * src/util/virstoragefile.c (virStorageFileGetMetadataInternal): > Don't nuke an already-existing encryption. > > Signed-off-by: Eric Blake > --- > src/util/virstoragefile.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c > index 43b7a5a..0792dd8 100644 > --- a/src/util/virstoragefile.c > +++ b/src/util/virstoragefile.c > @@ -805,7 +805,8 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr > meta, > > crypt_format = virReadBufInt32BE(buf + > > fileTypeInfo[meta->format].qcowCryptOffset); > -if (crypt_format && VIR_ALLOC(meta->encryption) < 0) > +if (crypt_format && !meta->encryption && > +VIR_ALLOC(meta->encryption) < 0) > goto cleanup; > } > ACK. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] QEMU: parse '-device vfio-pci' and '-device pci-assign'
Signed-off-by: Olivia Yin Signed-off-by: Laine Stump Modify the existing function qemuParseCommandLinePCI(), which works with -pcidevice, to support for "-device pci-assign" and "-device pci-assign". Change test cases 'hostdev-vfio' and 'hostdev-pci-address-device' to validate the new function. The case related to QEMU_CAPS_HOST_PCI_MULTIDOMAIN which uses 'host=domain:bus:slot.func' is not supported yet. --- src/qemu/qemu_command.c| 50 +++--- tests/qemuargv2xmltest.c | 3 +- .../qemuxml2argv-hostdev-pci-address-device.xml| 6 +++ .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 1 + 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e1d7e1b..3a4bc61 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10193,7 +10193,8 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt, * Tries to parse a QEMU PCI device */ static virDomainHostdevDefPtr -qemuParseCommandLinePCI(const char *val) +qemuParseCommandLinePCI(const char *val, +virDomainHostdevSubsysPCIBackendType backend) { int bus = 0, slot = 0, func = 0; const char *start; @@ -10222,10 +10223,20 @@ qemuParseCommandLinePCI(const char *val) goto error; } start = end + 1; -if (virStrToLong_i(start, NULL, 16, &func) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot extract PCI device function '%s'"), val); -goto error; + +if (backend) { +if (virStrToLong_i(start, &end, 16, &func) < 0 || *end != ',') { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device function '%s'"), val); +goto error; +} else +def->source.subsys.u.pci.backend = backend; +} else { +if (virStrToLong_i(start, NULL, 16, &func) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device function '%s'"), val); +goto error; +} } def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; @@ -11347,7 +11358,34 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } else if (STREQ(arg, "-pcidevice")) { virDomainHostdevDefPtr hostdev; WANT_VALUE(); -if (!(hostdev = qemuParseCommandLinePCI(val))) +if (!(hostdev = qemuParseCommandLinePCI(val, +VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT))) +goto error; +if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { +virDomainHostdevDefFree(hostdev); +goto error; +} +} else if (STREQ(arg, "-device") && progargv[i+1] && + STRPREFIX(progargv[i+1], "vfio-pci")) { +const char *start; +virDomainHostdevDefPtr hostdev; +WANT_VALUE(); +start = val + strlen("vfio-pci,"); +if (!(hostdev = qemuParseCommandLinePCI(start, +VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO))) +goto error; +if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { +virDomainHostdevDefFree(hostdev); +goto error; +} +} else if (STREQ(arg, "-device") && progargv[i+1] && + STRPREFIX(progargv[i+1], "pci-assign")) { +const char *start; +virDomainHostdevDefPtr hostdev; +WANT_VALUE(); +start = val + strlen("pci-assign,"); +if (!(hostdev = qemuParseCommandLinePCI(start, +VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM))) goto error; if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { virDomainHostdevDefFree(hostdev); diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 2cbbe3d..80188be 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -274,8 +274,9 @@ mymain(void) DO_TEST("watchdog"); DO_TEST("hostdev-usb-address"); - DO_TEST("hostdev-pci-address"); +DO_TEST("hostdev-pci-address-device"); +DO_TEST("hostdev-vfio"); DO_TEST("smp"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address-device.xml index b29ef58..b9a221a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address-device.xml @@ -15,10 +15,16 @@ /usr/bin/qemu + + + + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml index 8daa53a..b99f798 100644 --- a/tests/qemuxml2argv
Re: [libvirt] [patch v2 1/1] manual: Add virsh manual about specified migration host
Chen-san, Looks good to me. Thanks, > the 'migration_host' description maybe have a bit of difficulty to > understand for user, so add this manual for them. > > Signed-off-by: Chen Fan > --- > tools/virsh.pod | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/tools/virsh.pod b/tools/virsh.pod > index 02671b4..7b30292 100644 > --- a/tools/virsh.pod > +++ b/tools/virsh.pod > @@ -1208,7 +1208,8 @@ such as GFS2 or GPFS. If you are sure the migration is > safe or you just do not > care, use I<--unsafe> to force the migration. > > The I is the connection URI of the destination host, and > -I is the migration URI, which usually can be omitted (see below). > +I is the migration URI for specifying which IP address/URI of the > +destination host to tansfer migration data, which usually can be omitted > (see below). > I is used for renaming the domain to new name during migration, which > also usually can be omitted. Likewise, I<--xml> B is usually > omitted, but can be used to supply an alternative XML file for use on > @@ -1238,6 +1239,15 @@ seen from the source machine. > > When I is not specified, libvirt will automatically determine the > hypervisor specific URI, by looking up the target host's configured hostname. > + > +For QEMU/KVM hypervisor, when I is not specified, at first > libvirt > +will ask the destination side whether the optional "migration_host" is > specified > +or not, if the "migration_host" is specified, libvirt will use the specified > +network for transferring migration data(the "migrateion_host" is useful when > +hosts has multiple network interface). if the "migrateion_host" is not > specified > +too, libvirt will automatically determine the hypervisor specific URI, by > looking > +up the target host's configured hostname. > + > There are a few scenarios where specifying I may help: > > =over 4 > -- > 1.9.3 -- Yasunori Goto -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] SELinux: don't silently fail when no label is present
On 06/09/2014 08:30 AM, Ján Tomko wrote: > This fixes startup of a domain with: > > on a host with selinux and dac drivers and > security_default_confined = 0 > > https://bugzilla.redhat.com/show_bug.cgi?id=1105939 > --- > src/security/security_selinux.c | 98 > - > 1 file changed, 29 insertions(+), 69 deletions(-) Shouldn't there also be a change somewhere under tests/ to ensure that we parse XML in that situation? Otherwise, this looks like a fairly mechanical conversion of virDomainDefGetSecurityLabelDef() returning NULL from being a silent error into being successful; and that makes sense since domain_conf.c does not report any errors if a label cannot be looked up. ACK. -- 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
[libvirt] [PATCH] storage: fix memory leak with encrypted images
Jim Fehlig reported a regression found by libvirt-TCK tests: > ~ # perl /usr/share/libvirt-tck/tests/qemu/100-disk-encryption.t ... > ok 4 - defined persistent domain config > # Starting inactive domain config > libvirt error code: 1, message: internal error: unable to execute QEMU command > 'cont': 'drive-ide0-0-1' > (/var/cache/libvirt-tck/300-disk-encryption/demo.qcow2) is encrypted Commit 2279d560 converted a boolean into a pointer with the intent of transferring that pointer out of a temporary object into the caller's data structure. The temporary structure meant that meta->encryption was always NULL on entry, so we could get away with blindly allocating the pointer when the header said so. But later commits then tweaked things to do backing chain detection in-place, rather than via a temporary object; this has the net result that meta->encryption can be non-NULL on entry. Not only did this turn the latent behavior into a memory leak, it is also a behavior regression: blindly allocating a new pointer wipes out what secrets we already knew about the chain, making it impossible to restart the domain. Of course, no one in their right mind should be relying on qcow2 encryption - it is fundamentally flawed. And sadly, the TCK tests don't get run often enough, and this shows that our virstoragetest does not exercise encrypted images at all. Otherwise, we could have avoided a release containing this regression. * src/util/virstoragefile.c (virStorageFileGetMetadataInternal): Don't nuke an already-existing encryption. Signed-off-by: Eric Blake --- src/util/virstoragefile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 43b7a5a..0792dd8 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -805,7 +805,8 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta, crypt_format = virReadBufInt32BE(buf + fileTypeInfo[meta->format].qcowCryptOffset); -if (crypt_format && VIR_ALLOC(meta->encryption) < 0) +if (crypt_format && !meta->encryption && +VIR_ALLOC(meta->encryption) < 0) goto cleanup; } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] SELinux: don't silently fail when no label is present
This fixes startup of a domain with: on a host with selinux and dac drivers and security_default_confined = 0 https://bugzilla.redhat.com/show_bug.cgi?id=1105939 --- src/security/security_selinux.c | 98 - 1 file changed, 29 insertions(+), 69 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 8380bba..008c58c 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -585,7 +585,7 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr mgr, seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (seclabel == NULL) -return rc; +return 0; data = virSecurityManagerGetPrivateData(mgr); @@ -739,11 +739,7 @@ virSecuritySELinuxReserveSecurityLabel(virSecurityManagerPtr mgr, virSecurityLabelDefPtr seclabel; seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); -if (seclabel == NULL) { -return -1; -} - -if (seclabel->type == VIR_DOMAIN_SECLABEL_STATIC) +if (!seclabel || seclabel->type == VIR_DOMAIN_SECLABEL_STATIC) return 0; if (getpidcon_raw(pid, &pctx) == -1) { @@ -1060,7 +1056,7 @@ virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (seclabel == NULL) -return -1; +return 0; switch (tpm->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: @@ -1102,7 +1098,7 @@ virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr, seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (seclabel == NULL) -return -1; +return 0; switch (tpm->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: @@ -1136,7 +1132,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (seclabel == NULL) -return -1; +return 0; disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, SECURITY_SELINUX_NAME); @@ -1256,10 +1252,7 @@ virSecuritySELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, cbdata.manager = mgr; cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); -if (cbdata.secdef == NULL) -return -1; - -if (cbdata.secdef->norelabel) +if (!cbdata.secdef || cbdata.secdef->norelabel) return 0; if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) @@ -1279,7 +1272,7 @@ virSecuritySELinuxSetSecurityHostdevLabelHelper(const char *file, void *opaque) secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (secdef == NULL) -return -1; +return 0; return virSecuritySELinuxSetFilecon(file, secdef->imagelabel); } @@ -1397,7 +1390,7 @@ virSecuritySELinuxSetSecurityHostdevCapsLabel(virDomainDefPtr def, secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (secdef == NULL) -return -1; +return 0; switch (dev->source.caps.type) { case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE: { @@ -1447,10 +1440,7 @@ virSecuritySELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN virSecurityLabelDefPtr secdef; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); -if (secdef == NULL) -return -1; - -if (secdef->norelabel) +if (!secdef || secdef->norelabel) return 0; switch (dev->mode) { @@ -1635,10 +1625,7 @@ virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virSecurityLabelDefPtr secdef; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); -if (secdef == NULL) -return -1; - -if (secdef->norelabel) +if (!secdef || secdef->norelabel) return 0; switch (dev->mode) { @@ -1667,14 +1654,14 @@ virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def, int ret = -1; seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); -if (seclabel == NULL) -return -1; +if (!seclabel || seclabel->norelabel) +return 0; if (dev) chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev, SECURITY_SELINUX_NAME); -if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel)) +if (chr_seclabel && chr_seclabel->norelabel) return 0; if (chr_seclabel) @@ -1738,13 +1725,13 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virSecurityManagerPtr mgr, int ret = -1; seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); -if (seclabel == NULL) -return -1; +if (!seclabel || seclabel->norelabel) +return 0; if (dev)
[libvirt] QMP fallback race in libvirt
Hi, Eric asked me to move this here from #virt so it doesn't get forgotten. I hit a weird bug in a new install of libvirt on Debian Jessie this week where a vm could not be configured to use any CPU type except passthrough. After much digging and headscratching, the immediate cause for that turns out to be one of the (three) files in the qemu/capabilities cache being generated wrongly the first time that libvirtd was started. Instead of being generated from the QMP queries, it appears to have fallen back to the old method of scraping 'qemu -cpu help', and since the output of that changed with qemu 2.0 it leads to things like: and hilarity then ensues when cpuModelIsAllowed() is called by x86Decode(). Since only one of the cache files there was corrupted like this, it would appear libvirt either didn't wait long enough, or didn't try hard enough, to get a connection to the monitor for the QMP query on what was probably also the very first time qemu had been started on this host machine. After nuking the cache files and restarting libvirtd they were then correctly regenerated, and things began to work as expected. This was all done on a new clean install of the host machine, so there was nothing around from any earlier versions to get tangled up with, and possibly means qemu had some first time init of its own to do which took some time before it was ready to be queried. I am also seeing bursts of several of these warnings in the logs: libvirtd[26475]: This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous Which I haven't confirmed as being related, but doesn't seem to be obviously unrelated either, and at worst is a separate bug. Cheers, Ron -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 07/36] storage: Switch metadata crawler to use storage driver to get unique path
Peter Krempa wrote: > On 06/07/14 20:35, Roman Bogorodskiy wrote: > > Peter Krempa wrote: > > > >> Use the virStorageFileGetUniqueIdentifier() function to get a unique > >> identifier regardless of the target storage type instead of relying on > >> canonicalize_path(). > >> > >> A new function that checks whether we support a given image is > >> introduced to avoid errors for unimplemented backends. > >> --- > >> > >> Notes: > >> Version 3: > >> - fixed typo > >> - ACKed by Eric > >> > >> src/storage/storage_driver.c | 77 > >> +++- > >> 1 file changed, 54 insertions(+), 23 deletions(-) > >> > >> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > >> index f92a553..5c4188f 100644 > >> --- a/src/storage/storage_driver.c > >> +++ b/src/storage/storage_driver.c > > >> @@ -3112,49 +3135,63 @@ > >> virStorageFileGetMetadataRecurse(virStorageSourcePtr src, > >> int fd; > >> int ret = -1; > >> struct stat st; > >> +const char *uniqueName; > >> virStorageSourcePtr backingStore = NULL; > >> int backingFormat; > >> > >> -VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d > >> probe=%d", > >> - src->path, canonPath, NULLSTR(src->relDir), src->format, > >> +VIR_DEBUG("path=%s dir=%s format=%d uid=%d gid=%d probe=%d", > >> + src->path, NULLSTR(src->relDir), src->format, > >>(int)uid, (int)gid, allow_probe); > >> > >> -if (virHashLookup(cycle, canonPath)) { > >> -virReportError(VIR_ERR_INTERNAL_ERROR, > >> - _("backing store for %s is self-referential"), > >> - src->path); > >> +/* exit if we can't load information about the current image */ > >> +if (!virStorageFileSupportsBackingChainTraversal(src)) > >> +return 0; > > > > After this change I get virstrogetest failing on FreeBSD, which doesn't > > support any of the file storage backends currently: > > > > > > Program received signal SIGSEGV, Segmentation fault. > > [Switching to Thread 806c06400 (LWP 100061/virstoragetest)] > > 0x00410088 in mymain () at virstoragetest.c:937 > > 937 TEST_LOOKUP(3, "qcow2", chain->backingStore->path, > > chain->backingStore, > > Current language: auto; currently minimal > > (gdb) p chain > > $1 = 0x806c7b100 > > (gdb) p chain->backingStore > > $2 = 0x0 > > (gdb) > > > > > > Hmm, so FreeBSD doesn't currently compile the storage driver? We should > probably look into enabling it. All the stuff that was done by the code > was compiling just fine on FreeBSD previously so enabling just the local > filesystem storage backend should work well. I'll have a look what that > would include. virstorage.c test calls testStorageFileGetMetadata() which calls virStorageFileGetMetadata(). Inside of that, the call sequence is: virStorageFileGetMetadata -> virStorageFileGetMetadataRecurse virStorageFileGetMetadataRecurse then checks !virStorageFileSupportsBackingChainTraversal() and returns 0. virStorageFileSupportsBackingChainTraversal tries to initialise backend using virStorageFileBackendForTypeInternal(). virStorageFileBackendForTypeInternal loops through fileBackends which looks this way: static virStorageFileBackendPtr fileBackends[] = { #if WITH_STORAGE_FS &virStorageFileBackendFile, &virStorageFileBackendBlock, #endif #if WITH_STORAGE_GLUSTER &virStorageFileBackendGluster, #endif NULL }; FreeBSD doesn't support neither WITH_STORAGE_GLUSTER nor WITH_STORAGE_FS. So we end up having chain->backingStore == NULL. PS I may be wrong here as it's my first experience with the storage code and I haven't yet spend much time on it. Roman Bogorodskiy pgpFC6AqSWjaK.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] network: Bring netdevs online later
On 06/04/2014 05:55 PM, Matthew Rosato wrote: > Defer MAC registration until net devices are actually going > to be used by the guest. This patch does so by setting the > devices online just before starting guest CPUs. > > This approach is an alternative to my previously proposed > 'network: Defer online of macvtap during qemu migration' > Laine/Wangrui, is this the sort of thing you had in mind? Yes and no. It is basically what I was thinking - *always* wait to bring up the devices right before turning on the CPU of the guest. However, it doesn't account for hotplug - In that case, the CPUs have already been started, and the single device that has just been hotplugged needs to be started. This patch doesn't do anything about that. And there are a few other problems I saw from reading through it as well (this is without compiling/testing it): > > Previous thread: > https://www.redhat.com/archives/libvir-list/2014-May/msg00427.html > Associated BZ: > https://bugzilla.redhat.com/show_bug.cgi?id=1081461 > > Signed-off-by: Matthew Rosato > --- > src/qemu/qemu_command.c | 45 > +++ > src/qemu/qemu_command.h |3 +++ > src/qemu/qemu_process.c |3 +++ > src/util/virnetdevmacvlan.c |5 - > src/util/virnetdevtap.c |3 --- > 5 files changed, 51 insertions(+), 8 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index e6acced..c161d73 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -571,6 +571,51 @@ qemuNetworkPrepareDevices(virDomainDefPtr def) > return ret; > } > > +void > +qemuNetworkIfaceUp(virDomainNetDefPtr net) > +{ > +if (virNetDevSetOnline(net->ifname, true) < 0) { > +ignore_value(virNetDevTapDelete(net->ifname)); > +} > +return; > +} > + > +void > +qemuPhysIfaceUp(virDomainNetDefPtr net) > +{ > +if (virNetDevSetOnline(net->ifname, true) < 0) { > +ignore_value(virNetDevVPortProfileDisassociate(net->ifname, > + > virDomainNetGetActualVirtPortProfile(net), > + &net->mac, > + > virDomainNetGetActualDirectDev(net), > + -1, > + > VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)); Is this always the proper vmop (MIGRATE_IN_FINISH) no matter what the situation is? (remember it could now be happening not as the result of a failure during migration, but also as the result of a failure during the initial start of a domain, or during a hotplug). (I *really* dislike the way that this "vmop" (which is only used by low level macvtap functions) must be passed around through multiple layers of the callstack in higher level functions (existing problem), and wish/hope that there is a way to make it more localized, perhaps by storing the current state in the NetworkDef object as status somehow. But that's a separate issue, so for now we have to just continue passing it around.) > +ignore_value(virNetDevMacVLanDelete(net->ifname)); > +} > +return; > +} I think it would be better to have a single function that takes a virDomainNetPtr and contains the switch statement. This way 1) a call to it can easily be added in the proper place to support hotplug, and 2) that one function can later be moved to the same final location as what is currently called networkAllocateActualDevice() and those two functions can become part of an API that will allow non-privileged libvirt instances to use these network types. > + > +void > +qemuNetworkInitializeDevices(virDomainDefPtr def) I think the verb "Start" would be better than "Initialize" in this case - that one is too easily confused with the already existing "Prepare". Also, I think we should create a qemu_interface.c to contain all of these functions, similar to how we currently have a qemu_hostdev.c for functions related to . > +{ > +size_t i; > + > +for (i = 0; i < def->nnets; i++) { > +virDomainNetDefPtr net = def->nets[i]; > +switch(virDomainNetGetActualType(net)) { > +case VIR_DOMAIN_NET_TYPE_BRIDGE: > +case VIR_DOMAIN_NET_TYPE_NETWORK: > +qemuNetworkIfaceUp(net); > +break; > +case VIR_DOMAIN_NET_TYPE_DIRECT: > +qemuPhysIfaceUp(net); > +break; > +} > +} > + > +return; > +} > + > static int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, >const char *prefix) > { > diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h > index afbd6ff..4a44464 100644 > --- a/src/qemu/qemu_command.h > +++ b/src/qemu/qemu_command.h > @@ -206,6 +206,9 @@ int qemuOpenVhostNet(virDomainDefPtr def, > int *vhostfdSize); >
[libvirt] [PATCH] test: add user_xattr check for securityselinuxlabeltest
libvirt unit test used setxattr with "user.libvirt.selinux" name to emulate setfilecon of selinux. But for some old kernel filesystem (like 2.6.32-431.el6.x86_64), if the filesystem is not mounted with user_xattr flag, the setxattr with "user.libvirt.selinux" will fail. So adding testUserXattrEnabled() in securityselinuxlabeltest.c, if user_xattr is not enabled, skip this case. The user_xattr is departed in newer kernel, therefore this commit is only for the compatablity for old kernel. Signed-off-by: Jincheng Miao --- tests/securityselinuxlabeltest.c | 33 + 1 files changed, 33 insertions(+), 0 deletions(-) diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 88ec35a..3f155e3 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -28,6 +28,7 @@ #include #include +#include #include "internal.h" #include "testutils.h" @@ -56,6 +57,35 @@ struct testSELinuxFile { char *context; }; +static int +testUserXattrEnabled(void) +{ +int ret = -1; +ssize_t len; +const char *con_value = "system_u:object_r:svirt_image_t:s0:c41,c264"; +char *path = NULL; +if (virAsprintf(&path, "%s/securityselinuxlabeldata/testxattr", +abs_srcdir) < 0) +goto cleanup; + +if (virFileTouch(path, 0600) < 0) +goto cleanup; + +len = setxattr(path, "user.libvirt.selinux", con_value, + strlen(con_value), 0); +if (len < 0) { +if (errno == EOPNOTSUPP) +ret = 0; +goto cleanup; +} + +ret = 1; + + cleanup: +unlink(path); +VIR_FREE(path); +return ret; +} static int testSELinuxMungePath(char **path) @@ -322,6 +352,9 @@ mymain(void) { int ret = 0; +if (!testUserXattrEnabled()) +return EXIT_AM_SKIP; + if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false))) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Unable to initialize security driver: %s\n", -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: support interface type=network
On 06/07/2014 12:30 AM, Jim Fehlig wrote: > Add support for in the libxl driver. > > Signed-off-by: Jim Fehlig > --- > src/libxl/libxl_conf.c | 41 +++-- > 1 file changed, 39 insertions(+), 2 deletions(-) > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index cec37d6..6efcea6 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -908,7 +908,44 @@ libxlMakeNic(virDomainDefPtr def, > if (VIR_STRDUP(x_nic->script, l_nic->script) < 0) > return -1; > break; > -default: > +case VIR_DOMAIN_NET_TYPE_NETWORK: > +{ > +bool error = true; > +char *brname = NULL; > +virNetworkPtr network = NULL; > +virConnectPtr conn; > + > +if (!(conn = virConnectOpen("xen:///system"))) > +return -1; > + > +if (!(network = > + virNetworkLookupByName(conn, l_nic->data.network.name))) > +goto cleanup_net; > + > +if (!(brname = virNetworkGetBridgeName(network))) > +goto cleanup_net; This only accounts for the traditional libvirt-managed networks (the ones with or no at all, which use a transient bridge device created by libvirt). As long as you're adding this support, you may as well add it in a way that you can take advantage of the libvirt networks which are just thinly veiled coveres over existing bridges, e.g.: http://www.libvirt.org/formatnetwork.html#examplesBridge That can be done by following the example of qemunetworkIfaceConnect(). In short, instead of looking at l_nic->type, you look at virDomainNetGetActualType(l_nic), and do the above code only if *that* is VIR_DOMAIN_NET_TYPE_NETWORK. Otherwise, if actualType is VIR_DOMAIN_NET_TYPE_BRIDGE, you will want to VIR_STRDUP(x_nic->bridge, virDomainNetGetActualBridgeName(l_nic)) (note that this will end up accounting for both the case of an *AND* an where the network is a wrapper over a system-created bridge device). BTW, I notice that because you added the new case at the end, none of these network-based connections will use the script from the definition as is done with bridge-based connections (yet no error will be logged if one exists). Was this intentional? > + > +if (VIR_STRDUP(x_nic->bridge, brname) < 0) > +goto cleanup_net; > + > +error = false; > + > + cleanup_net: > +VIR_FREE(brname); > +virNetworkFree(network); > +virObjectUnref(conn); > +if (error) > +return -1; > +break; > +} > +case VIR_DOMAIN_NET_TYPE_USER: > +case VIR_DOMAIN_NET_TYPE_SERVER: > +case VIR_DOMAIN_NET_TYPE_CLIENT: > +case VIR_DOMAIN_NET_TYPE_MCAST: > +case VIR_DOMAIN_NET_TYPE_INTERNAL: > +case VIR_DOMAIN_NET_TYPE_DIRECT: > +case VIR_DOMAIN_NET_TYPE_HOSTDEV: > +case VIR_DOMAIN_NET_TYPE_LAST: > virReportError(VIR_ERR_INTERNAL_ERROR, > _("libxenlight does not support network device type %s"), > virDomainNetTypeToString(l_nic->type)); > @@ -919,7 +956,7 @@ libxlMakeNic(virDomainDefPtr def, > } > > static int > -libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) > +libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) > { > virDomainNetDefPtr *l_nics = def->nets; > size_t nnics = def->nnets; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] patches necessary to make the "parse vfio in commandline" work
On 06/06/2014 04:54 PM, Laine Stump wrote: > Patches 1/3 and 2/3 are prerequisites the the patch that started this > thread. Patch 3/3 should be squashed into the original patch. > > I also noticed that the original patch causes all unrecognized > "-device" options to now be ignored rather than being added to the > qemu namespace (with a warning). This needs to be fixed before > resubmitting that patch too, but I didn't have the time/interest to do > it. > > (If needed/desired, all three of these new patches can be pushed > separately before the patch at the top of this thread). > > Laine Stump (3): > test: display qemuParseCommandline warnings when VIR_TEST_DEBUG > 0 > qemu: ignore -nodefconfig and -nodefaults in > qemuParseCommandLineString > test: make hostdev-vfio test able to pass qemuargv2xmltest > > src/qemu/qemu_command.c| 4 +- > tests/qemuargv2xmltest.c | 45 > -- > .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 1 + > 3 files changed, 38 insertions(+), 12 deletions(-) > I pushed 1/3 and 2/3, and have sent another patch to add parsing for (which is also required for all the tests to patch once the original is done correctly). I left 3/3 to be squashed into the original patch of the thread. Thanks for the reviews! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] patches necessary to make the "parse vfio in commandline" work
On 06/06/2014 05:32 PM, Eric Blake wrote: > On 06/06/2014 07:54 AM, Laine Stump wrote: >> Patches 1/3 and 2/3 are prerequisites the the patch that started this >> thread. Patch 3/3 should be squashed into the original patch. >> >> I also noticed that the original patch causes all unrecognized >> "-device" options to now be ignored rather than being added to the >> qemu namespace (with a warning). This needs to be fixed before >> resubmitting that patch too, but I didn't have the time/interest to do >> it. >> >> (If needed/desired, all three of these new patches can be pushed >> separately before the patch at the top of this thread). > So for 3/3, which is it? Push now, or squash into the respin of the > patch that started this thread? I think it's too small, and not of any use by itself, so I recommended squashing it into the new version of the original patch that adds vfio-pci parsing. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] QEMU: Modify qemuParseCommandLinePCI() to parsee '-device vfio-pci'
On 06/09/2014 10:26 AM, Olivia Yin wrote: > Signed-off-by: Olivia Yin > > Modify qemuParseCommandLinePCI() to support parsing '-device > vfio-pci,host=bus:slot.func'. > Add test case 'hostdev-vfio' into qemuargv2xmltest to validate this function. > > The case related to QEMU_CAPS_HOST_PCI_MULTIDOMAIN which uses > '-device vfio-pci,host=domain:bus:slot.func' is not supported yet. 1) please squash in the change to the hostdev-vfio test data here (it's not of any use by itself): https://www.redhat.com/archives/libvir-list/2014-June/msg00372.html 2) As long as you're changing the single existing function to work for both -device vfio-pci and -pcidevice, you should also use it to add support for "-device pci-assign", which has identical syntax to "-device vfio-pci" (aside from the name, of course :-) 3) You are still causing unrecognized -device args to be silently discarded, but they should instead be added to the qemu namespace. See my comment inline below. > --- > src/qemu/qemu_command.c | 36 ++-- > tests/qemuargv2xmltest.c | 2 +- > 2 files changed, 31 insertions(+), 7 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 3cf279e..ae7f94e 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -10193,7 +10193,7 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt, > * Tries to parse a QEMU PCI device > */ > static virDomainHostdevDefPtr > -qemuParseCommandLinePCI(const char *val) > +qemuParseCommandLinePCI(const char *val, bool vfio) > { > int bus = 0, slot = 0, func = 0; > const char *start; > @@ -10222,10 +10222,20 @@ qemuParseCommandLinePCI(const char *val) > goto error; > } > start = end + 1; > -if (virStrToLong_i(start, NULL, 16, &func) < 0) { > -virReportError(VIR_ERR_INTERNAL_ERROR, > - _("cannot extract PCI device function '%s'"), val); > -goto error; > + > +if (!vfio) { > +if (virStrToLong_i(start, NULL, 16, &func) < 0) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot extract PCI device function '%s'"), > val); > +goto error; > +} > +} else { > +if (virStrToLong_i(start, &end, 16, &func) < 0 || *end != ',') { I don't think this difference between the two is needed. If you really want to get that nit-picky, you should be going all the way and using the helper function qemuParseKeywords to get *all* of the options, then cycling through them in a loop - it is totally legal for those options to come in any order, so not only might the host= option be the last (and thus it won't be followed by a ","), but there may be other options preceding it (for example the "bus", "addr", and "id" options, all used by libvirt). Your current parsing is tailored specifically to commandlines following the pattern used by libvirt, but the aim of qemuCommandLineParseString() is to parse commandlines from other sources as well. > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot extract PCI device function '%s'"), > val); > +goto error; > +} else > +def->source.subsys.u.pci.backend = > VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; > } > > def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; > @@ -11347,12 +11357,26 @@ qemuParseCommandLine(virCapsPtr qemuCaps, > } else if (STREQ(arg, "-pcidevice")) { > virDomainHostdevDefPtr hostdev; > WANT_VALUE(); > -if (!(hostdev = qemuParseCommandLinePCI(val))) > +if (!(hostdev = qemuParseCommandLinePCI(val,0))) Since the 2nd arg is a bool, you should send "true" or "false", not "1" and "0". And you need to put a space after any comma in an arglist. > goto error; > if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < > 0) { > virDomainHostdevDefFree(hostdev); > goto error; > } > +} else if (STREQ(arg, "-device")) { > +WANT_VALUE(); > +if (STRPREFIX(val, "vfio-pci,")) { > +const char *start; > +start = val; > +virDomainHostdevDefPtr hostdev; > +start += strlen("vfio-pci,"); > +if (!(hostdev = qemuParseCommandLinePCI(start,1))) Again, use "true" instead of "1", and don't forget the leading space. > +goto error; > +if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, > hostdev) < 0) { > +virDomainHostdevDefFree(hostdev); > +goto error; > +} > +} As I mentioned before, this code will cause any unrecognized -device to be ignored. Instead, you really want something like this: } else if (STREQ(arg, "-device") && progargv[i + 1] && STRPREFIX(progargv[i + 1],
[libvirt] [PATCH] qemu: parse -device virtio-balloon
There are no options to parse here, and all three possible device names have the same prefix ("virtio-balloon" with "-ccw", "-pci", or "-device" appended), so it is fairly simple. qemuParseCommandLineString() previously would always add a to every result (the comments erroneously say that it is adding a ) This has been changed to add model='none', and 84 test case xml's updated accordingly (so that qemuxml2argvtest won't fail). Now that the memballoon device is properly parsed, we can safely add a test for properly ignoring -nodefconfig and -nodefaults. Rather than adding an entire new test case for this (and memballoon), we just randomly pick the clock-utc test and modify it slightly to fulfill the purpose. --- I don't necessarily have any love for the memory balloon driver, but this ended up being necessary in order to prevent test failures after fixing the patch here to not ignore unrecognized -device args: https://www.redhat.com/archives/libvir-list/2014-June/msg00388.html src/qemu/qemu_command.c | 8 +++- tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-clock-utc.args| 7 --- tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-cdrom.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-disk.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-disk-drive-cache-directsync.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.xml| 2 +- .../qemuxml2argv-disk-drive-error-policy-enospace.xml | 2 +- .../qemuxml2argv-disk-drive-error-policy-stop.xml | 2 +- .../qemuxml2argv-disk-drive-error-policy-wreport-rignore.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-fmt-qcow.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml | 2 +- .../qemuxml2argv-disk-drive-network-nbd-export.xml| 2 +- .../qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml| 2 +- .../qemuxml2argv-disk-drive-network-rbd-ceph-env.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml| 2 +- .../qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-policy.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-websocket.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.xml
[libvirt] [patch v2 1/1] manual: Add virsh manual about specified migration host
the 'migration_host' description maybe have a bit of difficulty to understand for user, so add this manual for them. Signed-off-by: Chen Fan --- tools/virsh.pod | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 02671b4..7b30292 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1208,7 +1208,8 @@ such as GFS2 or GPFS. If you are sure the migration is safe or you just do not care, use I<--unsafe> to force the migration. The I is the connection URI of the destination host, and -I is the migration URI, which usually can be omitted (see below). +I is the migration URI for specifying which IP address/URI of the +destination host to tansfer migration data, which usually can be omitted (see below). I is used for renaming the domain to new name during migration, which also usually can be omitted. Likewise, I<--xml> B is usually omitted, but can be used to supply an alternative XML file for use on @@ -1238,6 +1239,15 @@ seen from the source machine. When I is not specified, libvirt will automatically determine the hypervisor specific URI, by looking up the target host's configured hostname. + +For QEMU/KVM hypervisor, when I is not specified, at first libvirt +will ask the destination side whether the optional "migration_host" is specified +or not, if the "migration_host" is specified, libvirt will use the specified +network for transferring migration data(the "migrateion_host" is useful when +hosts has multiple network interface). if the "migrateion_host" is not specified +too, libvirt will automatically determine the hypervisor specific URI, by looking +up the target host's configured hostname. + There are a few scenarios where specifying I may help: =over 4 -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vmware: make version parsing more robust
On Monday 28 April 2014 13:46:54 Ján Tomko wrote: > On 04/09/2014 11:59 AM, Jean-Baptiste Rouault wrote: > > Since commit d69415d4, vmware version is parsed from both stdout and > > stderr. This patch makes version parsing work even if there is garbage > > (libvirt debug messages for example) in the command output. > > For libvirt's debug messages, we have virLogProbablyLogMessage that can > check if the message matches libvirt's format. > > Should we really ignore all garbage? Honestly I don't know, but as of today the only garbage I saw there was from libvirt. Apart from the version string, I don't think there could be useful information in the command output. Regards, Jean-Baptiste -- Jean-Baptiste ROUAULT R&D Engineer - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] parallels: Avoid possible leak of "cpu" from parallelsBuildCapabilities
On 06/09/14 09:43, Martin Kletzander wrote: > On Mon, Jun 09, 2014 at 09:39:44AM +0200, Peter Krempa wrote: >> 4d06af97d38c3648937eb8f732704379b3cd9e59 introduced a possible memory >> leak of the memory allocated into the "cpu" pointer in >> parallelsBuildCapabilities in the case "nodeGetInfo()" would fail right >> after the allocation. Rearrange the code to avoid the possibility of the >> leak. >> >> Found by Coverity. >> --- >> src/parallels/parallels_driver.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> > > ACK, > > Martin > Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 07/36] storage: Switch metadata crawler to use storage driver to get unique path
On 06/07/14 20:35, Roman Bogorodskiy wrote: > Peter Krempa wrote: > >> Use the virStorageFileGetUniqueIdentifier() function to get a unique >> identifier regardless of the target storage type instead of relying on >> canonicalize_path(). >> >> A new function that checks whether we support a given image is >> introduced to avoid errors for unimplemented backends. >> --- >> >> Notes: >> Version 3: >> - fixed typo >> - ACKed by Eric >> >> src/storage/storage_driver.c | 77 >> +++- >> 1 file changed, 54 insertions(+), 23 deletions(-) >> >> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c >> index f92a553..5c4188f 100644 >> --- a/src/storage/storage_driver.c >> +++ b/src/storage/storage_driver.c >> @@ -3112,49 +3135,63 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr >> src, >> int fd; >> int ret = -1; >> struct stat st; >> +const char *uniqueName; >> virStorageSourcePtr backingStore = NULL; >> int backingFormat; >> >> -VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d >> probe=%d", >> - src->path, canonPath, NULLSTR(src->relDir), src->format, >> +VIR_DEBUG("path=%s dir=%s format=%d uid=%d gid=%d probe=%d", >> + src->path, NULLSTR(src->relDir), src->format, >>(int)uid, (int)gid, allow_probe); >> >> -if (virHashLookup(cycle, canonPath)) { >> -virReportError(VIR_ERR_INTERNAL_ERROR, >> - _("backing store for %s is self-referential"), >> - src->path); >> +/* exit if we can't load information about the current image */ >> +if (!virStorageFileSupportsBackingChainTraversal(src)) >> +return 0; > > After this change I get virstrogetest failing on FreeBSD, which doesn't > support any of the file storage backends currently: > > > Program received signal SIGSEGV, Segmentation fault. > [Switching to Thread 806c06400 (LWP 100061/virstoragetest)] > 0x00410088 in mymain () at virstoragetest.c:937 > 937 TEST_LOOKUP(3, "qcow2", chain->backingStore->path, > chain->backingStore, > Current language: auto; currently minimal > (gdb) p chain > $1 = 0x806c7b100 > (gdb) p chain->backingStore > $2 = 0x0 > (gdb) > > Hmm, so FreeBSD doesn't currently compile the storage driver? We should probably look into enabling it. All the stuff that was done by the code was compiling just fine on FreeBSD previously so enabling just the local filesystem storage backend should work well. I'll have a look what that would include. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Question about how to distinguish a usb device in usb pass-through feature
Hi, Qemu has supported 3rd method for USB passthrough except two ways that you have pointed: hostbus+hostport -- match for a specific physical port in the host, any device which is plugged in there gets passed to the guest. The method can resolve your all problems. AFAICT, libvirt do not support this way at present. Any Plan? CC'ing libvirt developer mailist. Best regards, -Gonglei From: qemu-devel-bounces+arei.gonglei=huawei@nongnu.org [mailto:qemu-devel-bounces+arei.gonglei=huawei@nongnu.org] On Behalf Of Yuanjing (D) Sent: Monday, June 09, 2014 11:37 AM To: qemu-de...@nongnu.org Cc: Liuji (Jeremy) Subject: [Qemu-devel] Question about how to distinguish a usb device in usb pass-through feature Hi, I want to provide feature that pass-through host's usb device to guest os in Openstack. I have question about how to distinguish a usb device. I have read some introductions and made some tests. I think qemu supports two ways to identify a host usb device: (1)host:bus.addr (2)host:vendor_id:product_id I think they both have restriction on some use cases: (1) 'host:bus.addr' is not appropriate if more than one usb devices with the same 'host:bus.addr' in a host. (2) The addr(device number) may change every time unplug/plug usb device of a host. One this happen the guest may reboot failed with wrong host:bus.addr. I noticed that Vmware supports technology 'autoconnection', they pass-through usb device by using usb path of the device on the host(physical topology and port location). Once unplug usb device and plug usb device again with the same usb path, the new device appears and is connected to the guest. I am not a qemu/system programmer and I apologize if I am missing something very obvious. Hope somebody can help me to find the best way to distinguish a usb device? Thanks in advance -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] m4: bhyve: Fix check for the required bhyve programs
On 06/06/14 17:58, Eric Blake wrote: > On 06/06/2014 09:27 AM, Peter Krempa wrote: >> bhyveload and bhyvectl wouldn't be checked otherwise as the configure >> script wouldn't execute one of the tests: >> >> checking for bhyve... /usr/local/sbin/bhyve >> checking for bhyvectl... /usr/local/sbin/bhyvectl >> checking for bhyveload... /usr/local/sbin/bhyveload >> ./configure: line 62602: test: too many arguments >> >> Fix the shell statement testing the 3 binaries. >> --- >> m4/virt-driver-bhyve.m4 | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > ACK. > Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] parallels: Avoid possible leak of "cpu" from parallelsBuildCapabilities
On Mon, Jun 09, 2014 at 09:39:44AM +0200, Peter Krempa wrote: 4d06af97d38c3648937eb8f732704379b3cd9e59 introduced a possible memory leak of the memory allocated into the "cpu" pointer in parallelsBuildCapabilities in the case "nodeGetInfo()" would fail right after the allocation. Rearrange the code to avoid the possibility of the leak. Found by Coverity. --- src/parallels/parallels_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK, Martin diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 153961b..411527c 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -151,10 +151,10 @@ parallelsBuildCapabilities(void) "parallels", NULL, NULL, 0, NULL) == NULL) goto error; -if (VIR_ALLOC(cpu) < 0) +if (nodeGetInfo(&nodeinfo)) goto error; -if (nodeGetInfo(&nodeinfo)) +if (VIR_ALLOC(cpu) < 0) goto error; cpu->arch = caps->host.arch; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] parallels: Avoid possible leak of "cpu" from parallelsBuildCapabilities
4d06af97d38c3648937eb8f732704379b3cd9e59 introduced a possible memory leak of the memory allocated into the "cpu" pointer in parallelsBuildCapabilities in the case "nodeGetInfo()" would fail right after the allocation. Rearrange the code to avoid the possibility of the leak. Found by Coverity. --- src/parallels/parallels_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 153961b..411527c 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -151,10 +151,10 @@ parallelsBuildCapabilities(void) "parallels", NULL, NULL, 0, NULL) == NULL) goto error; -if (VIR_ALLOC(cpu) < 0) +if (nodeGetInfo(&nodeinfo)) goto error; -if (nodeGetInfo(&nodeinfo)) +if (VIR_ALLOC(cpu) < 0) goto error; cpu->arch = caps->host.arch; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] QEMU: Modify qemuParseCommandLinePCI() to parsee '-device vfio-pci'
Signed-off-by: Olivia Yin Modify qemuParseCommandLinePCI() to support parsing '-device vfio-pci,host=bus:slot.func'. Add test case 'hostdev-vfio' into qemuargv2xmltest to validate this function. The case related to QEMU_CAPS_HOST_PCI_MULTIDOMAIN which uses '-device vfio-pci,host=domain:bus:slot.func' is not supported yet. --- src/qemu/qemu_command.c | 36 ++-- tests/qemuargv2xmltest.c | 2 +- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3cf279e..ae7f94e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10193,7 +10193,7 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt, * Tries to parse a QEMU PCI device */ static virDomainHostdevDefPtr -qemuParseCommandLinePCI(const char *val) +qemuParseCommandLinePCI(const char *val, bool vfio) { int bus = 0, slot = 0, func = 0; const char *start; @@ -10222,10 +10222,20 @@ qemuParseCommandLinePCI(const char *val) goto error; } start = end + 1; -if (virStrToLong_i(start, NULL, 16, &func) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot extract PCI device function '%s'"), val); -goto error; + +if (!vfio) { +if (virStrToLong_i(start, NULL, 16, &func) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device function '%s'"), val); +goto error; +} +} else { +if (virStrToLong_i(start, &end, 16, &func) < 0 || *end != ',') { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device function '%s'"), val); +goto error; +} else +def->source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; } def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; @@ -11347,12 +11357,26 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } else if (STREQ(arg, "-pcidevice")) { virDomainHostdevDefPtr hostdev; WANT_VALUE(); -if (!(hostdev = qemuParseCommandLinePCI(val))) +if (!(hostdev = qemuParseCommandLinePCI(val,0))) goto error; if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { virDomainHostdevDefFree(hostdev); goto error; } +} else if (STREQ(arg, "-device")) { +WANT_VALUE(); +if (STRPREFIX(val, "vfio-pci,")) { +const char *start; +start = val; +virDomainHostdevDefPtr hostdev; +start += strlen("vfio-pci,"); +if (!(hostdev = qemuParseCommandLinePCI(start,1))) +goto error; +if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { +virDomainHostdevDefFree(hostdev); +goto error; +} +} } else if (STREQ(arg, "-soundhw")) { const char *start; WANT_VALUE(); diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 0fc9fcb..b4ba97a 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -251,8 +251,8 @@ mymain(void) DO_TEST("watchdog"); DO_TEST("hostdev-usb-address"); - DO_TEST("hostdev-pci-address"); +DO_TEST("hostdev-vfio"); DO_TEST("smp"); -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list