[libvirt] [PATCHv2] QEMU: Modify qemuParseCommandLinePCI() to parsee '-device vfio-pci'

2014-06-09 Thread Olivia Yin 
Signed-off-by: Olivia Yin hong-hua@freescale.com

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

[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

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

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] [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] [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] [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] [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
RD 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

[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 chen.fan.f...@cn.fujitsu.com
---
 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 Idesturi is the connection URI of the destination host, and
-Imigrateuri is the migration URI, which usually can be omitted (see below).
+Imigrateuri 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).
 Idname is used for renaming the domain to new name during migration, which
 also usually can be omitted.  Likewise, I--xml Bfile 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 Imigrateuri 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 Imigrateuri 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 Imigrateuri may help:
 
 =over 4
-- 
1.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[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 memballoon
model='virtio'/ to every result (the comments erroneously say that it
is adding a memballoon model='none'/) 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 +-

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 hong-hua@freescale.com

 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], vfio-pci)) {
  const char *start;
  virDomainHostdevDefPtr hostdev;

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] [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
memballoon (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] libxl: support interface type=network

2014-06-09 Thread Laine Stump 
On 06/07/2014 12:30 AM, Jim Fehlig wrote:
 Add support for interface type='network' in the libxl driver.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  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 forward mode='nat|route' or no forward 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 interface type='bridge' *AND* an
interface type='network' 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

[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 jm...@redhat.com
---
 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 selinux/selinux.h
 #include selinux/context.h
+#include attr/xattr.h
 
 #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 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 mjros...@linux.vnet.ibm.com
 ---
  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 hostdev.


 +{
 +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);
  
  int qemuNetworkPrepareDevices(virDomainDefPtr def);
 +void

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

[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:

cpu name='SandyBridge  Intel Xeon E312xx (Sandy Bridge)'/

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

[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:
seclabel type='none' model='dac'/
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;

[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 ebl...@redhat.com
---
 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

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 chen.fan.f...@cn.fujitsu.com
 ---
  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 Idesturi is the connection URI of the destination host, and
 -Imigrateuri is the migration URI, which usually can be omitted (see below).
 +Imigrateuri 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).
  Idname is used for renaming the domain to new name during migration, which
  also usually can be omitted.  Likewise, I--xml Bfile 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 Imigrateuri 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 Imigrateuri 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 Imigrateuri may help:
  
  =over 4
 -- 
 1.9.3

-- 
Yasunori Goto y-g...@jp.fujitsu.com


--
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 hong-hua@freescale.com
Signed-off-by: Laine Stump la...@laine.org

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 @@
   devices
 emulator/usr/bin/qemu/emulator
 disk type='block' device='disk'
+  driver name='qemu' type='raw'/
   source dev='/dev/HostVG/QEMUGuest2'/
   target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
 /disk
+controller type='usb' index='0'/

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 pkre...@redhat.com
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 ebl...@redhat.com
 ---
  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