Re: [libvirt] [PATCH] storage: fix memory leak with encrypted images

2014-06-09 Thread Jim Fehlig
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'

2014-06-09 Thread Olivia Yin
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

2014-06-09 Thread Yasunori Goto
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

2014-06-09 Thread Eric Blake
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

2014-06-09 Thread Eric Blake
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

2014-06-09 Thread Ján Tomko
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

2014-06-09 Thread Ron

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

2014-06-09 Thread Roman Bogorodskiy
  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

2014-06-09 Thread Laine Stump
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

2014-06-09 Thread Jincheng Miao
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

2014-06-09 Thread Laine Stump
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

2014-06-09 Thread Laine Stump
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

2014-06-09 Thread Laine Stump
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'

2014-06-09 Thread Laine Stump
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

2014-06-09 Thread Laine Stump
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

2014-06-09 Thread Chen Fan
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

2014-06-09 Thread Jean-Baptiste Rouault
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

2014-06-09 Thread Peter Krempa
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

2014-06-09 Thread Peter Krempa
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

2014-06-09 Thread Gonglei (Arei)
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

2014-06-09 Thread Peter Krempa
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

2014-06-09 Thread Martin Kletzander

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

2014-06-09 Thread Peter Krempa
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'

2014-06-09 Thread Olivia Yin
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