[libvirt] [PATCH] Valid pool format type for logical volume pool should be auto and lvm2

2014-06-18 Thread Shanzhi Yu
The logical volume pool supports formats auto and lvm2

Signed-off-by: Shanzhi Yu s...@redhat.com
---
 src/conf/storage_conf.c | 2 +-
 src/conf/storage_conf.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 8b6fd79..3d61273 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -77,7 +77,7 @@ VIR_ENUM_IMPL(virStoragePoolFormatDisk,
 
 VIR_ENUM_IMPL(virStoragePoolFormatLogical,
   VIR_STORAGE_POOL_LOGICAL_LAST,
-  unknown, lvm2)
+  auto, lvm2)
 
 
 VIR_ENUM_IMPL(virStorageVolFormatDisk,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 04d99eb..2b08ac5 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -462,7 +462,7 @@ typedef enum {
 VIR_ENUM_DECL(virStoragePoolFormatDisk)
 
 typedef enum {
-VIR_STORAGE_POOL_LOGICAL_UNKNOWN = 0,
+VIR_STORAGE_POOL_LOGICAL_AUTO = 0,
 VIR_STORAGE_POOL_LOGICAL_LVM2 = 1,
 VIR_STORAGE_POOL_LOGICAL_LAST,
 } virStoragePoolFormatLogical;
-- 
1.9.3

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


Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

2014-06-18 Thread Ian Campbell
On Tue, 2014-06-17 at 12:40 -0600, Jim Fehlig wrote:
 Ian Campbell wrote:
  On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote:

  This function exists in Xen 4.2 as well, in libxl.h.


  Any ideas on how to handle this?  I'm not aware of an existing macro to
  check for func 'foo' defined in header 'bar'.  Is writing a custom macro
  along these lines a good solution?  A bad solution I tried was hacking
  the test to check libxl version via libxl_get_version_info(), but that
  API does not work if not running Xen.
  
 
  Given that it exists in everything from 4.2 onwards why do you need to
  check for it?

 
 Hrm, right.  I had the half-brained idea to use this to solve the
 failures I saw when testing this series against Xen 4.2
 
 https://www.redhat.com/archives/libvir-list/2014-June/msg00170.html
 
 I think the solution to that specific problem is to use Xen 4.2 config
 as the baseline.  But it got me thinking about the general problem you
 mentioned near the end of this mail
 
 https://www.redhat.com/archives/libvir-list/2014-June/msg00032.html
 
 With virJSONStringCompare in 1/1, Daniel provides a way to handle
 existence of new fields showing up in the json.  But what if I want to
 write a test where the expected data is not supported on earlier
 versions?   E.g. how would I add a test to check conversion of 'tpm
 ...' to 'vtpms: [ ]' and expect that to work when running 'make check'
 against a 4.2 libxl where vtpms were not yet supported?  I suppose each
 such test would have to probe for the feature it checks and skip if not
 found.

Right. You'd probably want to gate such a test case on the corresponding
LIBXL_HAVE_XXX #define from libxl.h.

Ian.

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


Re: [libvirt] [PATCH libvirt] xen: handle root= in xen-xm configuration files.

2014-06-18 Thread Ian Campbell
On Tue, 2014-06-17 at 16:24 +0100, Ian Campbell wrote:
 +if (xenXMConfigGetString(conf, extra, extra, NULL)  0)

This was subtly broken. The default needs to be .

-8--

From 539412a6deac8b928c82945d692ef20a49535d65 Mon Sep 17 00:00:00 2001
From: Ian Campbell ian.campb...@citrix.com
Date: Tue, 17 Jun 2014 15:48:48 +0100
Subject: [PATCH] xen: handle root= in xen-xm configuration files.

In addition to extra= xm supported a root= option which was supposed
to be incorporated into the final command line. Handle that for virsh
domxml-from-native xen-xm. Tested with the libxl backend.

Signed-off-by: Ian Campbell ian.campb...@citrix.com
---
v2: extra should default to .
---
 .gnulib|2 +-

--- WTF. I stripped this out of the patch shown below...

 src/xenxs/xen_xm.c |   14 +-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index b2db97d..745041b 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -339,6 +339,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 def-os.nBootDevs++;
 }
 } else {
+const char *extra, *root;
+
 if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 
 0)
 goto cleanup;
 if (xenXMConfigCopyStringOpt(conf, bootargs, 
def-os.bootloaderArgs)  0)
@@ -348,8 +350,18 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 goto cleanup;
 if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd)  0)
 goto cleanup;
-if (xenXMConfigCopyStringOpt(conf, extra, def-os.cmdline)  0)
+if (xenXMConfigGetString(conf, extra, extra, )  0)
+goto cleanup;
+if (xenXMConfigGetString(conf, root, root, NULL)  0)
 goto cleanup;
+
+if (root) {
+if (virAsprintf(def-os.cmdline, root=%s %s, root, extra)  0)
+goto cleanup;
+} else {
+if (VIR_STRDUP(def-os.cmdline, extra)  0)
+goto cleanup;
+}
 }
 
 if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
-- 
1.7.10.4



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


Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator

2014-06-18 Thread Daniel P. Berrange
On Tue, Jun 17, 2014 at 12:40:30PM -0600, Jim Fehlig wrote:
 Ian Campbell wrote:
  On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote:

  This function exists in Xen 4.2 as well, in libxl.h.


  Any ideas on how to handle this?  I'm not aware of an existing macro to
  check for func 'foo' defined in header 'bar'.  Is writing a custom macro
  along these lines a good solution?  A bad solution I tried was hacking
  the test to check libxl version via libxl_get_version_info(), but that
  API does not work if not running Xen.
  
 
  Given that it exists in everything from 4.2 onwards why do you need to
  check for it?

 
 Hrm, right.  I had the half-brained idea to use this to solve the
 failures I saw when testing this series against Xen 4.2
 
 https://www.redhat.com/archives/libvir-list/2014-June/msg00170.html
 
 I think the solution to that specific problem is to use Xen 4.2 config
 as the baseline.  But it got me thinking about the general problem you
 mentioned near the end of this mail
 
 https://www.redhat.com/archives/libvir-list/2014-June/msg00032.html
 
 With virJSONStringCompare in 1/1, Daniel provides a way to handle
 existence of new fields showing up in the json.  But what if I want to
 write a test where the expected data is not supported on earlier
 versions?   E.g. how would I add a test to check conversion of 'tpm
 ...' to 'vtpms: [ ]' and expect that to work when running 'make check'
 against a 4.2 libxl where vtpms were not yet supported?  I suppose each
 such test would have to probe for the feature it checks and skip if not
 found.

My last approach was to allow the JSON comparator to skip certain
types of missing data but I don't think that's sufficiently flexible
anymore. I'm thinking instead, that we could maintain a set of
context paths which designate things that are new in each version,
which would indicate things that should be skipped with old versions.

So we with 4.3, we'd list

  /c_info/driver_domain
  /c_info/pvh

and any other newly added fields. Then when running the test on 4.2
if the expected XML contained either of those paths, we'd skip them
when comparing against the actual JSON.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH v4] Only set SELinux seclabel if supported by the host.

2014-06-18 Thread Christophe Fergeau
Hi,

On Tue, Jun 17, 2014 at 04:01:53PM +0200, Cédric Bosdonnat wrote:
 This code depends on new API in libvirt-gconfig to extract the
 secmodels handled by the host.
 ---
  Diff to v3:
   * Added yet another missing g_object_unref.
   * Fixed the logic for supportsSelinux
  libvirt-sandbox/libvirt-sandbox-builder.c | 49 
 +++
  1 file changed, 43 insertions(+), 6 deletions(-)
 
 diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c 
 b/libvirt-sandbox/libvirt-sandbox-builder.c
 index 48b3acc..d6b5735 100644
 --- a/libvirt-sandbox/libvirt-sandbox-builder.c
 +++ b/libvirt-sandbox/libvirt-sandbox-builder.c
 @@ -322,12 +322,10 @@ static gboolean 
 gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *build
  return TRUE;
  }
  
 -
 -static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder 
 *builder G_GNUC_UNUSED,
 -GVirSandboxConfig 
 *config G_GNUC_UNUSED,
 -const gchar 
 *statedir G_GNUC_UNUSED,
 -GVirConfigDomain 
 *domain,
 -GError **error 
 G_GNUC_UNUSED)
 +static gboolean gvir_sandbox_builder_construct_security_selinux 
 (GVirSandboxBuilder *builder,
 + 
 GVirSandboxConfig *config,
 + 
 GVirConfigDomain *domain,
 + GError 
 **error)
  {
  GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new();
  const char *label = gvir_sandbox_config_get_security_label(config);
 @@ -360,6 +358,45 @@ static gboolean 
 gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
  return TRUE;


  }
  
 +static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder 
 *builder,
 +GVirSandboxConfig 
 *config,
 +const gchar 
 *statedir G_GNUC_UNUSED,
 +GVirConfigDomain 
 *domain,
 +GError **error)
 +{
 +GVirConnection *connection = 
 gvir_sandbox_builder_get_connection(builder);
 +GVirConfigCapabilities *configCapabilities;
 +GVirConfigCapabilitiesHost *hostCapabilities;
 +GList *secmodels, *iter;
 +gboolean supportsSelinux = FALSE;
 +
 +/* What security models are available on the host? */
 +if (!(configCapabilities = gvir_connection_get_capabilities(connection, 
 error))) {

Missing g_object_unref(connection); here too.

 +return FALSE;
 +}
 +
 +hostCapabilities = gvir_config_capabilities_get_host(configCapabilities);
 +
 +secmodels = 
 gvir_config_capabilities_host_get_secmodels(hostCapabilities);
 +for (iter = secmodels; iter != NULL; iter = iter-next) {
 +if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model(
 +GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter-data)), 
 selinux))
 +supportsSelinux = TRUE;
 +g_object_unref(iter-data);
 +}
 +
 +g_list_free(secmodels);
 +g_object_unref(hostCapabilities);
 +g_object_unref(configCapabilities);
 +g_object_unref(connection);
 +
 +if (supportsSelinux)
 +return gvir_sandbox_builder_construct_security_selinux(builder, 
 config,
 +   domain, 
 error);
 +
 +return TRUE;

Wondering whether this we should return FALSE when we did nothing
because we only support SELinux.

Patch is fine otherwise, I can squash these changes in before pushing if
you don't want to send yet another iteration ;)

Christophe


pgpIK4BjaJOW5.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] virsh: man: Fix examples and docs for virsh version

2014-06-18 Thread Peter Krempa
The version command now takes the --daemon parameter for a while and the
output example was pretty outdated.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1110673
---
 tools/virsh.pod | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 35cf878..b522798 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -206,23 +206,28 @@ option instead.  For example:

 quit this interactive terminal

-=item Bversion
+=item Bversion [I--daemon]

 Will print out the major version info about what this built from.
+If I--daemon is specified then the version of the libvirt daemon
+is included in the output.

 =over 4

 BExample

-Bvirsh version
-
-Compiled against library: libvir 0.0.6
-
-Using library: libvir 0.0.6
-
-Using API: Xen 3.0.0
-
-Running hypervisor: Xen 3.0.0
+ $ virsh version
+ Compiled against library: libvirt 1.2.3
+ Using library: libvirt 1.2.3
+ Using API: QEMU 1.2.3
+ Running hypervisor: QEMU 2.0.50
+
+ $ virsh version --daemon
+ Compiled against library: libvirt 1.2.3
+ Using library: libvirt 1.2.3
+ Using API: QEMU 1.2.3
+ Running hypervisor: QEMU 2.0.50
+ Running against daemon: 1.2.6

 =back

-- 
1.9.3

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


[libvirt] [PATCH 0/2] virsh: man: Tweak some old docs problems

2014-06-18 Thread Peter Krempa
Peter Krempa (2):
  virsh: man: Fix examples and docs for virsh version
  virsh: man: Correctly spell QEMU

 tools/virsh.pod | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

-- 
1.9.3

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


[libvirt] [PATCH 2/2] virsh: man: Correctly spell QEMU

2014-06-18 Thread Peter Krempa
s/QEemu/QEMU/g
---
 tools/virsh.pod | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index b522798..ceec1a0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -19,7 +19,7 @@ under the GNU Lesser General Public License. Virtualization 
of the
 Linux Operating System means the ability to run multiple instances of
 Operating Systems concurrently on a single hardware system where the
 basic resources are driven by a Linux instance. The library aims at
-providing a long term stable C API.  It currently supports Xen, QEmu,
+providing a long term stable C API.  It currently supports Xen, QEMU,
 KVM, LXC, OpenVZ, VirtualBox and VMware ESX.

 The basic structure of most virsh usage is:
@@ -260,11 +260,11 @@ this is used to connect to the local Xen hypervisor

 =item qemu:///system

-connect locally as root to the daemon supervising QEmu and KVM domains
+connect locally as root to the daemon supervising QEMU and KVM domains

 =item qemu:///session

-connect locally as a normal user to his own set of QEmu and KVM domains
+connect locally as a normal user to his own set of QEMU and KVM domains

 =item lxc:///

-- 
1.9.3

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


Re: [libvirt] [Xen-devel] [PATCH libvirt] xen: handle root= in xen-xm configuration files.

2014-06-18 Thread Ian Campbell
On Tue, 2014-06-17 at 15:36 -0600, Jim Fehlig wrote:
 Eric Blake wrote:
  On 06/17/2014 09:24 AM, Ian Campbell wrote:

  In addition to extra= xm supported a root= option which was supposed
  to be incorporated into the final command line. Handle that for virsh
  domxml-from-native xen-xm. Tested with the libxl backend.
 
  Signed-off-by: Ian Campbell ian.campb...@citrix.com
  ---
   .gnulib|2 +-
   src/xenxs/xen_xm.c |   14 +-
   2 files changed, 14 insertions(+), 2 deletions(-)
 
  diff --git a/.gnulib b/.gnulib
  index d55899f..e8e0eb6 16
  --- a/.gnulib
  +++ b/.gnulib
  @@ -1 +1 @@
  -Subproject commit d55899fd2c5794ac85ecb14d5e2f646a89e4b4dd
  +Subproject commit e8e0eb6bfb728685ec8d5afd924e41b18e9d928d
  
 
  Was the submodule bump intended?

No, sorry, I've no idea how that happened (/me curses git submodules yet
again).

Once I understand that, then this patch (minus
  the .gnulib bump) seems okay.

NB I just sent out a v2 -- extra should default to  not NULL for this
to work as intended.

 BTW, if cmdline contains root=, I noticed that domxml-to-native will
 put it in extra= instead of creating a root= entry.  E.g.
 cmdlineroot=/dev/bla foo=bar/cmdline converts to
 extra=root=/dev/bla foo=bar, which is still valid config so perhaps
 not such a big deal.

I think this is fine.

Personally I consider the root= stuff to be a weird wart, in that it
effectively exposes details of the Linux command line syntax in the
xm/xl cfg file. It's far better IMHO to ignore it and write root=foo in
the actual command line.

Ian.

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


Re: [libvirt] [PATCH 11/12] Expose IOMMU and VFIO capabilities from virCaps

2014-06-18 Thread Michal Privoznik

On 30.05.2014 11:11, Daniel P. Berrange wrote:

On Thu, May 29, 2014 at 10:32:45AM +0200, Michal Privoznik wrote:

This piece of information may be useful for management application to
decide if a domain with a device passthrough can be started on given
host or not.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---

Notes:
 I'm not very happy with the element names, but they're the best I
 could come up with so far. If you have any better suggestion I am
 all ears.

  docs/formatcaps.html.in  |  8 +++-
  docs/schemas/capability.rng  | 12 
  src/conf/capabilities.c  |  4 
  tests/capabilityschemadata/caps-qemu-kvm.xml |  2 ++
  tests/capabilityschemadata/caps-test.xml |  2 ++
  tests/capabilityschemadata/caps-test2.xml|  2 ++
  tests/capabilityschemadata/caps-test3.xml|  2 ++
  tests/xencapsdata/xen-i686-pae-hvm.xml   |  2 ++
  tests/xencapsdata/xen-i686-pae.xml   |  2 ++
  tests/xencapsdata/xen-i686.xml   |  2 ++
  tests/xencapsdata/xen-ia64-be-hvm.xml|  2 ++
  tests/xencapsdata/xen-ia64-be.xml|  2 ++
  tests/xencapsdata/xen-ia64-hvm.xml   |  2 ++
  tests/xencapsdata/xen-ia64.xml   |  2 ++
  tests/xencapsdata/xen-ppc64.xml  |  2 ++
  tests/xencapsdata/xen-x86_64-hvm.xml |  2 ++
  tests/xencapsdata/xen-x86_64.xml |  2 ++
  17 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index d060a5b..eb8c905 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -35,6 +35,8 @@ BIOS you will see/p
lt;suspend_disk/gt;
lt;suspend_hybrid/gt;
  lt;power_management/gt;
+lt;kvmgt;truelt;/kvmgt;
+lt;vfiogt;truelt;/vfiogt;
lt;/hostgt;/span

lt;!-- xen-3.0-x86_64 --gt;
@@ -78,7 +80,11 @@ BIOS you will see/p
Suspend-to-Disk (S4) and Hybrid-Suspend (a combination of S3
and S4). In case the host does not support
any such feature, then an empty lt;power_management/gt;
-  tag will be shown. /p
+  tag will be shown. Then, two elements
+  codelt;kvm/gt;/code and codelt;vfio/gt;/code
+  expose the fact, whether the host supports legacy device
+  passthrough with IOMMU cooperation or newer Virtual function
+  I/O./p
  pThe second block (in blue) indicates the paravirtualization
support of the Xen support, you will see the os_type of xen
to indicate a paravirtual kernel, then architecture
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index d2d9776..3b378eb 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -48,6 +48,18 @@
zeroOrMore
  ref name='secmodel'/
/zeroOrMore
+  element name='kvm'
+  choice
+  valuefalse/value
+  valuetrue/value
+  /choice
+  /element
+  element name='vfio'
+  choice
+  valuefalse/value
+  valuetrue/value
+  /choice
+  /element
  /element
/define

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 9561ba3..a91f37b 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -901,6 +901,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
  virBufferAddLit(buf, /secmodel\n);
  }

+/* KVM and VFIO features */
+virBufferAsprintf(buf, kvm%s/kvm\n, caps-host.legacyKVMPassthrough ? true 
: false);
+virBufferAsprintf(buf, vfio%s/vfio\n, caps-host.VFIOPassthrough ? true : 
false);


Ah, so this is missing from the previous patch.

In fact the splt between this  previous patch is a bit confusing.

I'd expect the previous patch to only change the src/conf/capabilities*
files and generic test cases. While this patch only change the driver
code and driver specific test cases.


So this XML is really trying to provide a list of the valid enumeration
options for the driver element of hostdev devices. This is quite a
common request for many domain XML elements, so I feel we ought to do
something that is not so ad-hoc here. When we get into this world of
providing info about supported options for devices, we really need to
be able to express this on a fine granularity that the capabilities
XML allows for.

For example, different emulator binaries will support different options,
as will many different architectures, or even different machine types of
virtualization types.

So it feels like we need a new API for this, that accepts info about
the machine we're trying to launch. eg

   char * virConnectGetEmulatorCapabilties(virConnectPtr conn,
   const char *emulatorbin,
   const char *machine,
   const char *virttype);


What's this virttype argument?



NB I didn't 

Re: [libvirt] [PATCH 0/2] virsh: man: Tweak some old docs problems

2014-06-18 Thread Ján Tomko
On 06/18/2014 11:28 AM, Peter Krempa wrote:
 Peter Krempa (2):
   virsh: man: Fix examples and docs for virsh version
   virsh: man: Correctly spell QEMU
 
  tools/virsh.pod | 29 +
  1 file changed, 17 insertions(+), 12 deletions(-)

ACK




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 11/12] Expose IOMMU and VFIO capabilities from virCaps

2014-06-18 Thread Daniel P. Berrange
On Wed, Jun 18, 2014 at 11:51:51AM +0200, Michal Privoznik wrote:
 On 30.05.2014 11:11, Daniel P. Berrange wrote:
 diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
 index 9561ba3..a91f37b 100644
 --- a/src/conf/capabilities.c
 +++ b/src/conf/capabilities.c
 @@ -901,6 +901,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
   virBufferAddLit(buf, /secmodel\n);
   }
 
 +/* KVM and VFIO features */
 +virBufferAsprintf(buf, kvm%s/kvm\n, 
 caps-host.legacyKVMPassthrough ? true : false);
 +virBufferAsprintf(buf, vfio%s/vfio\n, 
 caps-host.VFIOPassthrough ? true : false);
 
 Ah, so this is missing from the previous patch.
 
 In fact the splt between this  previous patch is a bit confusing.
 
 I'd expect the previous patch to only change the src/conf/capabilities*
 files and generic test cases. While this patch only change the driver
 code and driver specific test cases.
 
 
 So this XML is really trying to provide a list of the valid enumeration
 options for the driver element of hostdev devices. This is quite a
 common request for many domain XML elements, so I feel we ought to do
 something that is not so ad-hoc here. When we get into this world of
 providing info about supported options for devices, we really need to
 be able to express this on a fine granularity that the capabilities
 XML allows for.
 
 For example, different emulator binaries will support different options,
 as will many different architectures, or even different machine types of
 virtualization types.
 
 So it feels like we need a new API for this, that accepts info about
 the machine we're trying to launch. eg
 
char * virConnectGetEmulatorCapabilties(virConnectPtr conn,
const char *emulatorbin,
const char *machine,
const char *virttype);
 
 What's this virttype argument?

Matches the same attribute in domain XML - domain type='kvm|qemu|xen|uml|...'

 
 NB I didn't include 'architecture' since that's implicit in the
 emulatorbin chosen. The 'char *' return value would be an XML
 schema
 
 As for the XML schema, I haven't given it huge thought, but perhaps
 something that loosely mirrors the XML schema is desirable.
 
 the XML schema? You mean the capabilities XML schema?

I mean the new emulator XML schema should roughly follow the structure
of the domain XML schema.

so if domain XML has  domaindevicesdisk.../disk/devices/domain
then, we'd want to keep this kind of nesting when reporting info about
what the disk element supports for its various enums.

 
 So for the hostdev driver type enum I could imagine starting off with:
 
emulatorCapabilities
  path/usr/bin/qemyu-system-x86_64/path
  domainkvm/domain
  archx86_64/arch
  machinepc-1.0/machine
  devices
hostdev
   driver
 
 I find this misleading. I mean, why kvm and vfio is under
 devices/hostdev/driver?

IIUC, you were attempting to expose flags showing whether the host kernel
supports vfio or kvm PCI passthrough. Apps would then consume that to
decide what the hostdev mode='subsystem' type='pci'  for values of the
driver name='kvm|vfio|...' attribute. Of course that's only half the
problem because they also need to know if the QEMU binary supports the
'vfio' scheme too.

I'm saying that instead of having capabilities which represent what the
host kernel supports, we should directly expose information on whether
you can use the attribute in the driver element of the hostdev. This
captures both kernel and QEMU supportability.

 
 enum name=driver
 valuekvm/value
 valuevfio/value
 /enum
   /driver
/hostdev
  /devices
/emulatorCapabilities
 
 I would expect that the 'maxCpus' hack we stuffed into the existing
 capabilities XML would be added to this too eg vcpus max=120/
 at the top level
 
 So let me see if I understand correctly. To represent emulatorCapabilities
 in memory we need another structure, say:
 
 typedef struct _virEmulatorCapabilities virEmulatorCapabilities;
 typedef virEmulatorCapabilities *virEmulatorCapabilitiesPtr;
 struct _virEmulatorCapabilities {
 some values here
 };
 
 which will live in src/conf/capabilities.*. Can you shed a light on the
 structure internals and where it should be created? IIUC, it'll be created
 in the virConnectGetEmulatorCapabilties(), then formated and immediately
 disposed.

Ignore the existing capabilities XML and its code. This should be a new
XML schema that is completely separate giving information specifically
related to features of the emulator in question. We'd not use the existing
virCapabilities structure. Instead I'd expect that we directly populate
this new schema with info from the qemuCapsPtr object.

This mechanism will let us address this more general problem that both
libguestfs and virt-manager have repeatedly asked for - a way to 

Re: [libvirt] [PATCH 0/2] virsh: man: Tweak some old docs problems

2014-06-18 Thread Peter Krempa
On 06/18/14 12:06, Ján Tomko wrote:
 On 06/18/2014 11:28 AM, Peter Krempa wrote:
 Peter Krempa (2):
   virsh: man: Fix examples and docs for virsh version
   virsh: man: Correctly spell QEMU

  tools/virsh.pod | 29 +
  1 file changed, 17 insertions(+), 12 deletions(-)
 
 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 v4] Only set SELinux seclabel if supported by the host.

2014-06-18 Thread Cedric Bosdonnat
On Wed, 2014-06-18 at 11:11 +0200, Christophe Fergeau wrote:
 Hi,
 
 On Tue, Jun 17, 2014 at 04:01:53PM +0200, Cédric Bosdonnat wrote:
  This code depends on new API in libvirt-gconfig to extract the
  secmodels handled by the host.
  ---
   Diff to v3:
* Added yet another missing g_object_unref.
* Fixed the logic for supportsSelinux
   libvirt-sandbox/libvirt-sandbox-builder.c | 49 
  +++
   1 file changed, 43 insertions(+), 6 deletions(-)
  
  diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c 
  b/libvirt-sandbox/libvirt-sandbox-builder.c
  index 48b3acc..d6b5735 100644
  --- a/libvirt-sandbox/libvirt-sandbox-builder.c
  +++ b/libvirt-sandbox/libvirt-sandbox-builder.c
  @@ -322,12 +322,10 @@ static gboolean 
  gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *build
   return TRUE;
   }
   
  -
  -static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder 
  *builder G_GNUC_UNUSED,
  -GVirSandboxConfig 
  *config G_GNUC_UNUSED,
  -const gchar 
  *statedir G_GNUC_UNUSED,
  -GVirConfigDomain 
  *domain,
  -GError **error 
  G_GNUC_UNUSED)
  +static gboolean gvir_sandbox_builder_construct_security_selinux 
  (GVirSandboxBuilder *builder,
  + 
  GVirSandboxConfig *config,
  + 
  GVirConfigDomain *domain,
  + GError 
  **error)
   {
   GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new();
   const char *label = gvir_sandbox_config_get_security_label(config);
  @@ -360,6 +358,45 @@ static gboolean 
  gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
   return TRUE;
 
 
   }
   
  +static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder 
  *builder,
  +GVirSandboxConfig 
  *config,
  +const gchar 
  *statedir G_GNUC_UNUSED,
  +GVirConfigDomain 
  *domain,
  +GError **error)
  +{
  +GVirConnection *connection = 
  gvir_sandbox_builder_get_connection(builder);
  +GVirConfigCapabilities *configCapabilities;
  +GVirConfigCapabilitiesHost *hostCapabilities;
  +GList *secmodels, *iter;
  +gboolean supportsSelinux = FALSE;
  +
  +/* What security models are available on the host? */
  +if (!(configCapabilities = 
  gvir_connection_get_capabilities(connection, error))) {
 
 Missing g_object_unref(connection); here too.

Oops, I forgot that one case indeed.

  +return FALSE;
  +}
  +
  +hostCapabilities = 
  gvir_config_capabilities_get_host(configCapabilities);
  +
  +secmodels = 
  gvir_config_capabilities_host_get_secmodels(hostCapabilities);
  +for (iter = secmodels; iter != NULL; iter = iter-next) {
  +if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model(
  +GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter-data)), 
  selinux))
  +supportsSelinux = TRUE;
  +g_object_unref(iter-data);
  +}
  +
  +g_list_free(secmodels);
  +g_object_unref(hostCapabilities);
  +g_object_unref(configCapabilities);
  +g_object_unref(connection);
  +
  +if (supportsSelinux)
  +return gvir_sandbox_builder_construct_security_selinux(builder, 
  config,
  +   domain, 
  error);
  +
  +return TRUE;
 
 Wondering whether this we should return FALSE when we did nothing
 because we only support SELinux.

No idea what the original intent was... but we shouldn't fail if we just
not using any security label: that may be a valid use case.

 Patch is fine otherwise, I can squash these changes in before pushing if
 you don't want to send yet another iteration ;)

Well, that would save a few mails to the mailing list ;)

--
Cedric

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

Re: [libvirt] NL_RECV RETURNED WITH ERROR: NO BUFFER SPACE AVAILABLE ERROR FOR IXGBE LOAD

2014-06-18 Thread Laine Stump
On 06/17/2014 06:16 AM, vaughan wrote:
 Hi experts,

 Release: OL7
 Kernel: 3.10.0-121.el7.x86_64

 Noticed below error on OL7 server, while loading Intel 10gigabit nic
 driver module , ixgbe  in syslog
 
 journal: nl_recv returned with error: No buffer space available
 ---

 Complete syslog content for ixgbe module load :

 un 16 20:46:10 ca-ostest432 kernel: ixgbe: Intel(R) 10 Gigabit PCI
 Express Network Driver - version 3.15.1-k
 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe: Copyright (c) 1999-2013
 Intel Corporation.
 Jun 16 20:46:10 ca-ostest432 kvm: 1 guest now active
 Jun 16 20:46:10 ca-ostest432 kvm: 0 guests now active
 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: Multiqueue
 Enabled: Rx Queue count = 16, Tx Queue count = 16
 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: (PCI
 Express:5.0GT/s:Width x8) 00:1b:21:c8:24:74
 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: MAC: 2, PHY: 9,
 SFP+: 3, PBA No: E70856-007
 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: PCI Express
 bandwidth of 32GT/s available
 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: (Speed:5.0GT/s,
 Width: x8, Encoding Loss:20%)
 Jun 16 20:46:10 ca-ostest432 journal: nl_recv returned with error: No
 buffer space available

A very similar problem (probably the same, but you're showing the kernel
error message rather than the error logged by libvirt) was reported and
addressed in libnl3 quite awhile back. libnl3 originally set the default
buffer size to 4096, which wasn't enough for SRIOV cards with lots of
VFs. So they increased it to 4 * 4096, which should be plenty for
anybody. That libnl3 patch is present in RHEL7.0 (currently at 3.2.21.6).

Can you verify the version of libnl3 you are running, and that it
contains this code

   if (page_size == 0)
  page_size = getpagesize() * 4;

in the function lib/nl.c:nl_recv() (previously it was just page_size =
getpagesize();). If you don't have that patch in your libnl3 package,
please backport the upstream commit that makes that change. If you do
have that patch in your libnl3, perhaps you have gotten a different
ixgbe driver from somewhere (we did test against ixgbe with the maximum
number of VFs, so there would have to be something different in your
driver). It would be good to figure out the source of the problem before
applying any fix anywhere - much better to understand the cause, and
right now I don't think we do; what is creating the need for such a
large buffer in your case, but not for others who use the same driver
with the same number of VFs?).


Up to now our position has been that this problem should be fixed in
libnl, so we have preferred to not patch libvirt for it, but instead get
libnl fixed. If we do decide to patch libvirt, I think it would be
better to turn on message peeking for nl_recv
(nl_socket_enable_msg_peek()), as that would solve the problem totally
and permanently (the upstream maintainer of libnl is reluctant to turn
that on by default due to potential performance problems in other users
of libnl)

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


[libvirt] [PATCH 0/2] fix issues in vbox found by make check on RHEL6

2014-06-18 Thread Pavel Hrdina
Pavel Hrdina (2):
  tests: fix vbox snapshot xmls
  vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'

 src/vbox/vbox_snapshot_conf.c  |   2 +-
 tests/vboxsnapshotxmldata/2disks-1snap.vbox| 292 +++
 tests/vboxsnapshotxmldata/2disks-2snap.vbox| 584 +++---
 .../vboxsnapshotxmldata/2disks-3snap-brother.vbox  | 292 +++
 tests/vboxsnapshotxmldata/2disks-3snap.vbox| 876 ++---
 5 files changed, 1023 insertions(+), 1023 deletions(-)

-- 
1.8.5.5

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


[libvirt] [PATCH 2/2] vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'

2014-06-18 Thread Pavel Hrdina
The function 'xmlSaveFormatFileEnc' has a last option to set
if you want to format dumped xml with whitespaces or not.

Older libxml2, the one used in RHEL6, take this option as it is
but newer libxml2 check this option if it's true or not. This
small difference somehow makes things messy on RHEL6 and generated
xml had extra new line and extra whitespaces.

We should pass 1 instead if -1 because the -1 confuses the libxml2.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 src/vbox/vbox_snapshot_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
index 3f7ad78..7af1231 100644
--- a/src/vbox/vbox_snapshot_conf.c
+++ b/src/vbox/vbox_snapshot_conf.c
@@ -1211,7 +1211,7 @@ 
virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachinePtr machine,
 }
 }
 
-if (xmlSaveFormatFileEnc(filePath, xml, ISO-8859-1, -1)  0) {
+if (xmlSaveFormatFileEnc(filePath, xml, ISO-8859-1, 1)  0) {
 virReportError(VIR_ERR_XML_ERROR, %s,
_(Unable to save the xml));
 goto cleanup;
-- 
1.8.5.5

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


Re: [libvirt] [PATCH v7 RFC 0/2] libxl USB prototype and design discussion

2014-06-18 Thread Ian Campbell
On Mon, 2014-06-02 at 14:44 +0100, George Dunlap wrote:
 == Open questions ==
 
 Those are things I think I know; there are a couple of pertinent
 factual questions which I'm not sure of:
 
 * Is it possible to specify PVUSB controllers and attach USB devices
 to them before the guest is up and running (i.e., before the frontend
 is connected)?  It looks like xend had a syntax for specifying virtual
 controllers and attaching virtual devices, so it seems like it should
 be possible.

Unless the PVUSB drivers are radically different to other PV devices
this ought to be possible and should just work. (Essentially you just
preload the backend with all the settings and htey get handled when the
f.e. turns up)

 * Is it possible to connect a USB 1.1 device to a PVUSB controller
 which has been specified 2.0, or would there have to be a separate
 virtual controller for each?

I'm a bit surprised that PVUSB exposes the concept of a version to the
FE at all. I suppose there is some USBish reason why the f.e. would
care.

But I don't know the answer to your question.

 * Is it possible for the toolstack to tell if dom0 (or whatever the
 specified backend domain) has PVUSB support before starting the guest?

After creating the nodes with state == XenbusStateInitialising (1) the
toolstack waits for the backend to move to XenbusStateInitWait (2)
before continuing, with a timeout. So you will detect this in a
controlled way. You can't tell before try the setup though since the
driver might be autoloaded.

(Assuming pvusbback is the same as everything else)

 * Is it possible for the toolstack to tell if domU has a working and
 connected PVUSB front-end?

It can observe the state variable being 4 I suppose. Why do you need to
know?

 * Do we want to be able to create virtual hubs for qemu-backed
 controllers at some point in the future?  Is there any groundwork we
 want to lay for that?

qemu-backed emulated or PV controllers?

I don't think emulated would make sense for a PV guest and if qemu
wanted to be a PV backend it would have to implement the usual xenstore
watches etc.

I suppose a backend type a la libxl_device_disk's = phy|tap|qdisk might
be needed for this, but I think you can probably add that in a
compatible way in the future if necessary.

 == Design questions ==
 
 Then based on that, we have several design questions.
 
 * How much of the controller thing should be exposed via libxl?  Via xl?
 
 * This series only allows you to specify the protocol, either PV or
 DEVICE_MODEL.  Would it be better, for instance, to get rid of that,
 and instead allow the user to specify the creation of USB controllers,
 allow them to have a type of HCI (or emulated) or PV, and allow
 the user to specify which controller to attach a specific device to?
 
 * How much smarts should we have in the libxl / xl about creating
 emulated/virtual controllers and of what kinds?
 
 * How much / what kind of smarts should be in libxl / xl about
 choosing a controller to plug a device into?

Dunno * 4. Your proposed design looked ok to me.

 * What about config file syntax?  Should we try to reuse / extend the
 current config syntax, or create a new one?  Should the new one allow
 the specification of controllers?  Should it perhaps *require* the
 specification of controllers?

We should at least strive for any existing xm config files which use USB
to continue working, but that needn't imply that the preferred xl syntax
looks that way.

Of course if the xm syntax is horribly terribly broken then we might
make a concious choice not to carry it forward, but the default should
be compatibility.


 For reference, here are some example config snippets from the current
 xl / xm config files:
 
 -- snip --
 # HVM USB
 usb=1
 usbdevice=['tablet','host:4.3']
 
 # HVM USB, not compatible with the above
 usbversion=3
 
 # xend's PVUSB config file; this creates one virtual controller, then
 # plugs hostdev 1.8 into port 1
 vusb=['usbver=2, numports=4, port_1=1-8']

Oh my. That last one is quite exciting.

 -- snip --
 
 Given that, here is a potential config file format:
 
 -- snip --
 # Create two controllers, one pv, one emulated
 usbcontroller=['type=pv,name=pv0,usbversion=2,numports=4',
'type=emul,name=hci0,usbversion=2']
 
 # Create a controller with the defaults; PV for PV guests, emul for HVM guests
 usbcontroller=[''] 
 
 # Same as above, but defaulting to version 2
 usbversion=2 
 
 # I think we should be able to automatically detect which format to
 # use; so I think we should re-use usbdevice.  I could be convinced otherwise.
 usbdevice=['type=tablet','type=hostdev,hostaddr=4.3,bus=pv0']

Does this require that the usbcontroller have been specified?

I think it would be good if xl would by default create some number of
appropriate controllers without my having to specify them explicitly,
iow just saying usbdevice=[...] should be enough.

I'm not saying that you shouldn't support more specific syntax for
people who want 

Re: [libvirt] [PATCH 1/2] tests: fix vbox snapshot xmls

2014-06-18 Thread Peter Krempa
On 06/18/14 14:29, Pavel Hrdina wrote:
 On RHEL6 the vboxsnapshotxmltest fails because of wrong xml that
 is generated by libvirt. However the core issue is in the xml data
 itself with the wrong indentation.
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
  tests/vboxsnapshotxmldata/2disks-1snap.vbox| 292 +++
  tests/vboxsnapshotxmldata/2disks-2snap.vbox| 584 +++---
  .../vboxsnapshotxmldata/2disks-3snap-brother.vbox  | 292 +++
  tests/vboxsnapshotxmldata/2disks-3snap.vbox| 876 
 ++---
  4 files changed, 1022 insertions(+), 1022 deletions(-)
 

ACK, git show -b verifies that it's a whitespace only change

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 2/2] vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'

2014-06-18 Thread Peter Krempa
On 06/18/14 14:29, Pavel Hrdina wrote:
 The function 'xmlSaveFormatFileEnc' has a last option to set
 if you want to format dumped xml with whitespaces or not.
 
 Older libxml2, the one used in RHEL6, take this option as it is
 but newer libxml2 check this option if it's true or not. This
 small difference somehow makes things messy on RHEL6 and generated
 xml had extra new line and extra whitespaces.
 
 We should pass 1 instead if -1 because the -1 confuses the libxml2.
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
  src/vbox/vbox_snapshot_conf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 

ACK,

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 v7 RFC 0/2] libxl USB prototype and design discussion

2014-06-18 Thread George Dunlap

On 06/18/2014 01:57 PM, Ian Campbell wrote:

* Is it possible for the toolstack to tell if domU has a working and
connected PVUSB front-end?

It can observe the state variable being 4 I suppose. Why do you need to
know?


It might be nice to be able to create both a pv and an emulated 
controller in the config file, and then have xl usb-attach [foo] to 
automatically plug it into the PV controller if the PV frontend seems to 
be up, and into the emulated controller if it doesn't seem to be up.


But that doesn't change the elements of the interface, just what the 
default would be if the controller field is empty.



* Do we want to be able to create virtual hubs for qemu-backed
controllers at some point in the future?  Is there any groundwork we
want to lay for that?

qemu-backed emulated or PV controllers?

I don't think emulated would make sense for a PV guest and if qemu
wanted to be a PV backend it would have to implement the usual xenstore
watches etc.


I mean, emulate an actual USB hub -- you know, it plugs into your USB 
controller and you can plug other USB devices into it.


I'm inclined not to bother with it at this point.


-- snip --

Given that, here is a potential config file format:

-- snip --
# Create two controllers, one pv, one emulated
usbcontroller=['type=pv,name=pv0,usbversion=2,numports=4',
'type=emul,name=hci0,usbversion=2']

# Create a controller with the defaults; PV for PV guests, emul for HVM guests
usbcontroller=['']

# Same as above, but defaulting to version 2
usbversion=2

# I think we should be able to automatically detect which format to
# use; so I think we should re-use usbdevice.  I could be convinced otherwise.
usbdevice=['type=tablet','type=hostdev,hostaddr=4.3,bus=pv0']

Does this require that the usbcontroller have been specified?

I think it would be good if xl would by default create some number of
appropriate controllers without my having to specify them explicitly,
iow just saying usbdevice=[...] should be enough.

I'm not saying that you shouldn't support more specific syntax for
people who want more control, just that it shouldn't be required to do
so.

(I'm just talking xl here, at the libxl layer I think it would be fine
to require them to be explicit).


That makes sense.


* Rather than having to specify a controller, automatically hot-plug
controllers as-needed.

At the xl level I think this would be good.


OK, sounds good.  Thanks for the input.

 -George

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


Re: [libvirt] [PATCH 2/2] vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'

2014-06-18 Thread Pavel Hrdina
On 18.6.2014 15:11, Peter Krempa wrote:
 On 06/18/14 14:29, Pavel Hrdina wrote:
 The function 'xmlSaveFormatFileEnc' has a last option to set
 if you want to format dumped xml with whitespaces or not.

 Older libxml2, the one used in RHEL6, take this option as it is
 but newer libxml2 check this option if it's true or not. This
 small difference somehow makes things messy on RHEL6 and generated
 xml had extra new line and extra whitespaces.

 We should pass 1 instead if -1 because the -1 confuses the libxml2.

 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
  src/vbox/vbox_snapshot_conf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 
 ACK,
 
 Peter
 
 

Thanks, pushed

Pavel

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


Re: [libvirt] [PATCH v7 RFC 0/2] libxl USB prototype and design discussion

2014-06-18 Thread George Dunlap

On 06/18/2014 01:57 PM, Ian Campbell wrote:

* Is it possible to connect a USB 1.1 device to a PVUSB controller
which has been specified 2.0, or would there have to be a separate
virtual controller for each?

I'm a bit surprised that PVUSB exposes the concept of a version to the
FE at all. I suppose there is some USBish reason why the f.e. would
care.

But I don't know the answer to your question.


Simon,

Can you put Test different USB version devices with different PVUSB bus 
version numbers on your to-do list?


(i.e., find USB 1.1, 2.0, and 3.0 devices, and try each of them with 
usbversion set to 1, 2, and 3).


Thanks!

 -George

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


Re: [libvirt] RFC: network interface tags vs. portgroups

2014-06-18 Thread Laine Stump
Ping - any opinions on this?

On 06/10/2014 01:01 PM, Laine Stump wrote:
 A couple releases ago (commit 7d5bf484, first appeared in 1.2.2) I
 modified the domain interface status xml to show what resources are
 actually in use for an interface, superseding the interface config in
 the cases where they conflict with each other.

 In particular, if there is an interface of type='network' that
 references a portgroup of the network in the source element, the
 interface status will not contain a source element showing the network
 and portgroup names, but instead the source resulting from applying the
 config is shown. For example, given the following domain interface and
 network definitions:


 interface type='network'
   source network='mynet' portgroup='xyzzy'/
   ...
 /interface


 network
   namemynet/name
   forward mode='bridge'/\
   bridge name='br0'/
   portgroup name='xyzzy'
 bandwidth
   inbound average='1000' peak='5000' floor='200' burst='1024'/
   outbound average='128' peak='256' burst='256'/
 /bandwidth
   /portgroup
 /network

 the status that was previously displayed when the domain was running
 would be identical to the config above (except that it would also
 contain the tap device name and alias). But now the status will be this:


 interface type='bridge'
   source bridge='br0'/
   bandwidth
 inbound average='1000' peak='5000' floor='200' burst='1024'/
 outbound average='128' peak='256' burst='256'/
   /bandwidth
   ...
 /interface

 The advantage here is that a hook script for the domain will be able to
 see the bandwidth (and vlan and physical device, if any) that are
 actually being used by the domain's interface. Because the config and
 status both use the same elements/attributes, we can only show one or
 the other; the thinking was that normally the status will be what is
 desired, and anyone who really wants to look at the config should use
 the VIR_DOMAIN_XML_INACTIVE flag when calling virDomainGetXMLDesc().

 As you would expect, a few months later (after the release of 1.2.4)
 someone on IRC checked in with a problem caused by this change - they
 had been using the portgroup name in the source element of the
 interface to determine what action to take during migration; they didn't
 even have any libvirt config stored in the portgroup, but were just
 using its name as a tag. Since the portgroup name is only a part of the
 source element when the interface is type='network', they now don't
 have a tag in the xml to use for their decision (and since they aren't
 explicitly calling virDomainGetXMLDesc() themselves, they can't simply
 get the INACTIVE xml to solve their problem).

 This use of a portgroup name struck me as potentially useful (although
 it is a slight hijacking of the original intent of portgroups), so I
 would like to restore that functionality. I came up with a couple
 different ways to solve the problem, and am looking for opinions before
 I spend any time on either.

 Solution 1:

 My initial thought was to just restore the portgroup name in the status
 XML; that could be done by moving the portgroup name out of the
 network-specific part of the object and into common data for all
 interface types (this way it could show up in the source element no
 matter what is the network type). However, once we've done that it
 becomes enticing to allow specification of a portgroup even in cases
 where the interface type != network; in those cases though, the
 portgroup would be *only* a tag to be used by external entities; this
 would lead to lax checking for existence of the specified portgroup, and
 may end up with people misspelling a portgroup name, then mistakenly
 believing that (e.g.) they had a bandwidth limit applied to a domain
 interface when none was in fact in effect. (alternately, we could allow
 it only if the interface *config* was for type='network', but that seems
 somehow logically broken, and you can bet that eventually someone would
 ask for us to allow it for all types)

 Solution 2:

 An alternate idea I had was to add a new tag name='x'/ element to
 interfaces, networks, and portgroups. An interface could have multiple
 tags, and would assume the tags of its network when active. A tag
 would be purely for use by external entities - it would mean nothing to
 libvirt. For example, given this extreme example:

 interface type='network'
   source network='mynet' portgroup='xyzzy'/
   tag name='wumpus'/
   ...
 /interface

 network
   namemynet/name
   tag name='twisty'/
   forward mode='bridge'/\
   bridge name='br0'/
   portgroup name='xyzzy'
 tag name='xyzzytag'/
 bandwidth
   inbound average='1000' peak='5000' floor='200' burst='1024'/
   outbound average='128' peak='256' burst='256'/
 /bandwidth
   /portgroup
 /network
network

 when the 

[libvirt] [php PATCH] Use long variable type for zend_parse_parameters.

2014-06-18 Thread Dawid Zamirski
This patch fixes a bug where zend_parse_parameters would segfault on
certain PHP version (spotted on PHP 5.3.2 64bit) where type specifier is
long l and variable reference is int or unsigned int. Changing the
variable type from int or unsigned int to long fixes the problem for me
and is a known issue [1]. This did not happen in newer PHP version
(5.3.10).

[1] https://bugs.php.net/bug.php?id=59289
---
 src/libvirt-php.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 07ae137..224943d 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -3283,8 +3283,8 @@ PHP_FUNCTION(libvirt_domain_get_metadata)
 {
php_libvirt_domain *domain = NULL;
zval *zdomain;
-   int type = 0;
-   unsigned int flags = 0;
+   long type = 0;
+   long flags = 0;
char *uri = NULL;
int uri_len;
char *ret = NULL;
@@ -3328,8 +3328,8 @@ PHP_FUNCTION(libvirt_domain_set_metadata)
char *metadata = NULL;
char *key = NULL;
char *uri = NULL;
-   int type = 0;
-   unsigned int flags = 0;
+   long type = 0;
+   long flags = 0;
int rc;
 
GET_DOMAIN_FROM_ARGS (rlsssl, zdomain, type, metadata, 
metadata_len, key, key_len, uri, uri_len, flags);
@@ -3566,7 +3566,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api)
 {
php_libvirt_domain *domain=NULL;
zval *zdomain;
-   unsigned int screen = 0;
+   long screen = 0;
int fd = -1;
char file[] = /tmp/libvirt-php-tmp-XX;
virStreamPtr st = NULL;
@@ -3668,7 +3668,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot)
int port = -1;
char *hostname = NULL;
int hostname_len;
-   int scancode = 10;
+   long scancode = 10;
char *path;
char name[1024] = { 0 };
int use_builtin = 0;
@@ -3908,9 +3908,9 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event)
char *xml = NULL;
char *hostname = NULL;
int hostname_len;
-   int pos_x = 0;
-   int pos_y = 0;
-   int clicked = 0;
+   long pos_x = 0;
+   long pos_y = 0;
+   long clicked = 0;
int release = 1;
int ret;
 
@@ -4352,12 +4352,12 @@ PHP_FUNCTION(libvirt_domain_new)
// char *emulator;
char *iso_image = NULL;
int iso_image_len;
-   int vcpus = -1;
-   int memMB = -1;
+   long vcpus = -1;
+   long memMB = -1;
zval *disks, *networks;
tVMDisk *vmDisks = NULL;
tVMNetwork *vmNetworks = NULL;
-   int maxmemMB = -1;
+   long maxmemMB = -1;
HashTable *arr_hash;
int numDisks, numNets, i;
zval **data; // removed **zvalue
@@ -4642,7 +4642,7 @@ PHP_FUNCTION(libvirt_domain_get_interface_devices)
 */
 PHP_FUNCTION(libvirt_domain_change_vcpus)
 {
-   int numCpus, flags = 0;
+   long numCpus, flags = 0;
php_libvirt_domain *domain=NULL;
zval *zdomain;
 
@@ -6266,7 +6266,7 @@ PHP_FUNCTION(libvirt_domain_snapshot_delete)
 {
zval *zsnapshot;
php_libvirt_snapshot *snapshot;
-   int flags = 0;
+   long flags = 0;
int retval;
 
GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags);
@@ -6766,7 +6766,7 @@ PHP_FUNCTION(libvirt_storagevolume_delete)
 {
php_libvirt_volume *volume=NULL;
zval *zvolume;
-   int flags = 0;
+   long flags = 0;
int retval = 0;
 
GET_VOLUME_FROM_ARGS(r|l,zvolume,flags);
@@ -7057,7 +7057,7 @@ PHP_FUNCTION(libvirt_storagepool_refresh)
 {
php_libvirt_storagepool *pool = NULL;
zval *zpool;
-   unsigned long flags = 0;
+   long flags = 0;
int retval;
 
GET_STORAGEPOOL_FROM_ARGS (r|l, zpool, flags);
-- 
1.9.3

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


Re: [libvirt] [PATCH libvirt] xen: handle root= in xen-xm configuration files.

2014-06-18 Thread Jim Fehlig
Ian Campbell wrote:
 On Tue, 2014-06-17 at 16:24 +0100, Ian Campbell wrote:
   
 +if (xenXMConfigGetString(conf, extra, extra, NULL)  0)
 

 This was subtly broken. The default needs to be .

 -8--

 From 539412a6deac8b928c82945d692ef20a49535d65 Mon Sep 17 00:00:00 2001
 From: Ian Campbell ian.campb...@citrix.com
 Date: Tue, 17 Jun 2014 15:48:48 +0100
 Subject: [PATCH] xen: handle root= in xen-xm configuration files.

 In addition to extra= xm supported a root= option which was supposed
 to be incorporated into the final command line. Handle that for virsh
 domxml-from-native xen-xm. Tested with the libxl backend.

 Signed-off-by: Ian Campbell ian.campb...@citrix.com
 ---
 v2: extra should default to .
 ---
  .gnulib|2 +-

 --- WTF. I stripped this out of the patch shown below...

  src/xenxs/xen_xm.c |   14 +-
  2 files changed, 14 insertions(+), 2 deletions(-)
   

ACK and pushed; thanks!

Regards,
Jim

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index b2db97d..745041b 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -339,6 +339,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  def-os.nBootDevs++;
  }
  } else {
 +const char *extra, *root;
 +
  if (xenXMConfigCopyStringOpt(conf, bootloader, 
 def-os.bootloader)  0)
  goto cleanup;
  if (xenXMConfigCopyStringOpt(conf, bootargs, 
 def-os.bootloaderArgs)  0)
 @@ -348,8 +350,18 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  goto cleanup;
  if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd)  0)
  goto cleanup;
 -if (xenXMConfigCopyStringOpt(conf, extra, def-os.cmdline)  0)
 +if (xenXMConfigGetString(conf, extra, extra, )  0)
 +goto cleanup;
 +if (xenXMConfigGetString(conf, root, root, NULL)  0)
  goto cleanup;
 +
 +if (root) {
 +if (virAsprintf(def-os.cmdline, root=%s %s, root, extra)  0)
 +goto cleanup;
 +} else {
 +if (VIR_STRDUP(def-os.cmdline, extra)  0)
 +goto cleanup;
 +}
  }
  
  if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
   

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


Re: [libvirt] [PATCH python] Correct virDomainMigrateToURI3 definition

2014-06-18 Thread Jason Andryuk
Ping?

(Adding Daniel Veillard, listed as owner for libvirt-python.git, as this was 
previously just sent to the list.)

On 6/5/2014 11:43 AM, Jason Andryuk wrote:
 dconnuri is a string, so update the definition to match.  Without this,
 the generated python would fail when passed a string.
 ---
   libvirt-override-api.xml | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
 index d5b25b5..935e04d 100644
 --- a/libvirt-override-api.xml
 +++ b/libvirt-override-api.xml
 @@ -562,7 +562,7 @@
 infoMigrate the domain object from its current host to the 
 destination host
   given by URI./info
 arg name='domain' type='virDomainPtr' info='a domain object'/
 -  arg name='dconnuri' type='virConnectPtr' info='URI for target 
 libvirtd if @flags includes VIR_MIGRATE_PEER2PEER'/
 +  arg name='dconnuri' type='char *' info='URI for target libvirtd if 
 @flags includes VIR_MIGRATE_PEER2PEER'/
 arg name='params' type='char *' info='dictionary with migration 
 parameters'/
 arg name='flags' type='unsigned int' info='an ORapos;ed set of 
 virDomainMigrateFlags'/
 return type='int' info='0 in case of success, -1 in case of 
 failure.'/
 

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


Re: [libvirt] [PATCH] Valid pool format type for logical volume pool should be auto and lvm2

2014-06-18 Thread John Ferlan


On 06/18/2014 03:25 AM, Shanzhi Yu wrote:
 The logical volume pool supports formats auto and lvm2
 
 Signed-off-by: Shanzhi Yu s...@redhat.com
 ---
  src/conf/storage_conf.c | 2 +-
  src/conf/storage_conf.h | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index 8b6fd79..3d61273 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -77,7 +77,7 @@ VIR_ENUM_IMPL(virStoragePoolFormatDisk,
  
  VIR_ENUM_IMPL(virStoragePoolFormatLogical,
VIR_STORAGE_POOL_LOGICAL_LAST,
 -  unknown, lvm2)
 +  auto, lvm2)
  
  
  VIR_ENUM_IMPL(virStorageVolFormatDisk,
 diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
 index 04d99eb..2b08ac5 100644
 --- a/src/conf/storage_conf.h
 +++ b/src/conf/storage_conf.h
 @@ -462,7 +462,7 @@ typedef enum {
  VIR_ENUM_DECL(virStoragePoolFormatDisk)
  
  typedef enum {
 -VIR_STORAGE_POOL_LOGICAL_UNKNOWN = 0,
 +VIR_STORAGE_POOL_LOGICAL_AUTO = 0,
  VIR_STORAGE_POOL_LOGICAL_LVM2 = 1,
  VIR_STORAGE_POOL_LOGICAL_LAST,
  } virStoragePoolFormatLogical;
 

NACK

See http://libvirt.org/storage.html for details on how 'auto' is used.

Although not specifically called out for Logical Volume pools in the
documentation - it follows how other pools (filesystem and network
filesystem) use auto.

Also remember how libvirt works - 'calloc' memory resulting in a
'default' of 0 (zero).  Without the presence of a format type, it'll
be up to the pool code to determine what's there and what to default to.
There's only one format type for Logical Volume pools - so it's probably
fairly easy to guess what's in the pool...

John

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


Re: [libvirt] [php PATCH] Use long variable type for zend_parse_parameters.

2014-06-18 Thread Michal Novotny
Hi Dawid,

I've applied it to the libvirt-php git repository.

See:
http://libvirt.org/git/?p=libvirt-php.git;a=commit;h=d3b3afa7d37541984d1e80e4ab46cd3e582ea60d

Thanks,
Michal


2014-06-18 19:11 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com:


2014-06-18 19:11 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com:

 This patch fixes a bug where zend_parse_parameters would segfault on
 certain PHP version (spotted on PHP 5.3.2 64bit) where type specifier is
 long l and variable reference is int or unsigned int. Changing the
 variable type from int or unsigned int to long fixes the problem for me
 and is a known issue [1]. This did not happen in newer PHP version
 (5.3.10).

 [1] https://bugs.php.net/bug.php?id=59289
 ---
  src/libvirt-php.c | 32 
  1 file changed, 16 insertions(+), 16 deletions(-)

 diff --git a/src/libvirt-php.c b/src/libvirt-php.c
 index 07ae137..224943d 100644
 --- a/src/libvirt-php.c
 +++ b/src/libvirt-php.c
 @@ -3283,8 +3283,8 @@ PHP_FUNCTION(libvirt_domain_get_metadata)
  {
 php_libvirt_domain *domain = NULL;
 zval *zdomain;
 -   int type = 0;
 -   unsigned int flags = 0;
 +   long type = 0;
 +   long flags = 0;
 char *uri = NULL;
 int uri_len;
 char *ret = NULL;
 @@ -3328,8 +3328,8 @@ PHP_FUNCTION(libvirt_domain_set_metadata)
 char *metadata = NULL;
 char *key = NULL;
 char *uri = NULL;
 -   int type = 0;
 -   unsigned int flags = 0;
 +   long type = 0;
 +   long flags = 0;
 int rc;

 GET_DOMAIN_FROM_ARGS (rlsssl, zdomain, type, metadata,
 metadata_len, key, key_len, uri, uri_len, flags);
 @@ -3566,7 +3566,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api)
  {
 php_libvirt_domain *domain=NULL;
 zval *zdomain;
 -   unsigned int screen = 0;
 +   long screen = 0;
 int fd = -1;
 char file[] = /tmp/libvirt-php-tmp-XX;
 virStreamPtr st = NULL;
 @@ -3668,7 +3668,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot)
 int port = -1;
 char *hostname = NULL;
 int hostname_len;
 -   int scancode = 10;
 +   long scancode = 10;
 char *path;
 char name[1024] = { 0 };
 int use_builtin = 0;
 @@ -3908,9 +3908,9 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event)
 char *xml = NULL;
 char *hostname = NULL;
 int hostname_len;
 -   int pos_x = 0;
 -   int pos_y = 0;
 -   int clicked = 0;
 +   long pos_x = 0;
 +   long pos_y = 0;
 +   long clicked = 0;
 int release = 1;
 int ret;

 @@ -4352,12 +4352,12 @@ PHP_FUNCTION(libvirt_domain_new)
 // char *emulator;
 char *iso_image = NULL;
 int iso_image_len;
 -   int vcpus = -1;
 -   int memMB = -1;
 +   long vcpus = -1;
 +   long memMB = -1;
 zval *disks, *networks;
 tVMDisk *vmDisks = NULL;
 tVMNetwork *vmNetworks = NULL;
 -   int maxmemMB = -1;
 +   long maxmemMB = -1;
 HashTable *arr_hash;
 int numDisks, numNets, i;
 zval **data; // removed **zvalue
 @@ -4642,7 +4642,7 @@ PHP_FUNCTION(libvirt_domain_get_interface_devices)
  */
  PHP_FUNCTION(libvirt_domain_change_vcpus)
  {
 -   int numCpus, flags = 0;
 +   long numCpus, flags = 0;
 php_libvirt_domain *domain=NULL;
 zval *zdomain;

 @@ -6266,7 +6266,7 @@ PHP_FUNCTION(libvirt_domain_snapshot_delete)
  {
 zval *zsnapshot;
 php_libvirt_snapshot *snapshot;
 -   int flags = 0;
 +   long flags = 0;
 int retval;

 GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags);
 @@ -6766,7 +6766,7 @@ PHP_FUNCTION(libvirt_storagevolume_delete)
  {
 php_libvirt_volume *volume=NULL;
 zval *zvolume;
 -   int flags = 0;
 +   long flags = 0;
 int retval = 0;

 GET_VOLUME_FROM_ARGS(r|l,zvolume,flags);
 @@ -7057,7 +7057,7 @@ PHP_FUNCTION(libvirt_storagepool_refresh)
  {
 php_libvirt_storagepool *pool = NULL;
 zval *zpool;
 -   unsigned long flags = 0;
 +   long flags = 0;
 int retval;

 GET_STORAGEPOOL_FROM_ARGS (r|l, zpool, flags);
 --
 1.9.3

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

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

[libvirt] [php PATCH] Fix compiler warnings after int to long conversion

2014-06-18 Thread Dawid Zamirski
The previous patch [1] caused compiler warnings after variable types
were changed from int to long and this patch fixes this.

[1] https://www.redhat.com/archives/libvir-list/2014-June/msg00835.html
---
 src/libvirt-php.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 224943d..6d6fa81 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -3928,7 +3928,7 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event)
RETURN_FALSE;
}
 
-   DPRINTF(%s: x = %d, y = %d, clicked = %d, release = %d, hostname = 
%s...\n, PHPFUNC, pos_x, pos_y, clicked, release, hostname);
+   DPRINTF(%s: x = %d, y = %d, clicked = %d, release = %d, hostname = 
%s...\n, PHPFUNC, (int) pos_x, (int) pos_y, (int) clicked, release, hostname);
ret = vnc_send_pointer_event(hostname, tmp, pos_x, pos_y, clicked, 
release);
if (ret == 0) {
DPRINTF(%s: Pointer event result is %d\n, PHPFUNC, ret);
@@ -4428,7 +4428,7 @@ PHP_FUNCTION(libvirt_domain_new)
numNets = i;
 
snprintf(tmpname, sizeof(tmpname), %s-install, name);
-   DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d\n, PHPFUNC, 
tmpname, memMB, maxmemMB);
+   DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d\n, PHPFUNC, 
tmpname, (int) memMB, (int) maxmemMB);
tmp = installation_get_xml(1,
conn-conn, tmpname, memMB, maxmemMB, NULL /* arch */, 
NULL, vcpus, iso_image,
vmDisks, numDisks, vmNetworks, numNets,
@@ -6272,7 +6272,7 @@ PHP_FUNCTION(libvirt_domain_snapshot_delete)
GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags);
 
retval = virDomainSnapshotDelete(snapshot-snapshot, flags);
-   DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned %d\n, PHPFUNC, 
snapshot-snapshot, flags, retval);
+   DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned %d\n, PHPFUNC, 
snapshot-snapshot, (int) flags, retval);
if (retval == -1) RETURN_FALSE;
RETURN_TRUE;
 }
@@ -6772,7 +6772,7 @@ PHP_FUNCTION(libvirt_storagevolume_delete)
GET_VOLUME_FROM_ARGS(r|l,zvolume,flags);
 
retval = virStorageVolDelete(volume-volume, flags);
-   DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d\n, PHPFUNC, 
volume-volume, flags, retval);
+   DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d\n, PHPFUNC, 
volume-volume, (int) flags, retval);
 if (retval != 0) {
set_error_if_unset(Cannot delete storage volume TSRMLS_CC);
RETURN_FALSE;
-- 
1.9.3

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


[libvirt] [python PATCH] build: provide wrapper makefile

2014-06-18 Thread Eric Blake
After years of finger training, I'm so used to 'make check' just
working, that I lose quite a bit of time re-learning that in this
project, it is spelled 'python setup.py build check'.  A shim
makefile bridges the gap.

* Makefile: New file.

Signed-off-by: Eric Blake ebl...@redhat.com
---

I'd like to add this to the repo, but even if it gets rejected,
I'll still keep it in my local tree :)

 Makefile | 18 ++
 1 file changed, 18 insertions(+)
 create mode 100644 Makefile

diff --git a/Makefile b/Makefile
new file mode 100644
index 000..6c8da0a
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,18 @@
+# Shim wrapper around setup.py to allow for familiar build targets
+
+PYTHON ?= python
+
+all:
+   $(PYTHON) setup.py build
+
+install: all
+   $(PYTHON) setup.py install
+
+clean:
+   $(PYTHON) setup.py clean
+
+check: all
+   $(PYTHON) setup.py test
+
+rpm:
+   $(PYTHON) setup.py rpm
-- 
1.9.3

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


Re: [libvirt] [python PATCH] build: provide wrapper makefile

2014-06-18 Thread Nehal J Wani
On Thu, Jun 19, 2014 at 12:51 AM, Eric Blake ebl...@redhat.com wrote:
 After years of finger training, I'm so used to 'make check' just
 working, that I lose quite a bit of time re-learning that in this
 project, it is spelled 'python setup.py build check'.  A shim
 makefile bridges the gap.

 * Makefile: New file.

I see no harm in adding this. Definitely makes a developer's life easier. :)


 Signed-off-by: Eric Blake ebl...@redhat.com
 ---

 I'd like to add this to the repo, but even if it gets rejected,
 I'll still keep it in my local tree :)

  Makefile | 18 ++
  1 file changed, 18 insertions(+)
  create mode 100644 Makefile

 diff --git a/Makefile b/Makefile
 new file mode 100644
 index 000..6c8da0a
 --- /dev/null
 +++ b/Makefile
 @@ -0,0 +1,18 @@
 +# Shim wrapper around setup.py to allow for familiar build targets
 +
 +PYTHON ?= python
 +
 +all:
 +   $(PYTHON) setup.py build
 +
 +install: all
 +   $(PYTHON) setup.py install
 +
 +clean:
 +   $(PYTHON) setup.py clean
 +
 +check: all
 +   $(PYTHON) setup.py test
 +
 +rpm:
 +   $(PYTHON) setup.py rpm
 --
 1.9.3

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



-- 
Nehal J Wani

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


Re: [libvirt] [php PATCH] Fix compiler warnings after int to long conversion

2014-06-18 Thread Michal Novotny
Hi Dawid,
thanks for the information. Well, please rebase to the latest commit of my
master branch and resend, thanks a lot!

Michal


2014-06-18 22:05 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com:

 Ugh, now I know what happened - my local master branch has commits I
 have not sent pull requests for yet so it wasn't in 100% in sync with
 remote. Do you want me to resend the patch to the ML?

 On Wed, 2014-06-18 at 16:02 -0400, Dawid Zamirski wrote:
  Hi Michal,
 
  I'm pretty sure I did git pull right before sending the patch. Here's
  what I did exactly:
 
  On master branch:
 
  git pull
  git checkout -b parse-param-fix origin/master
  created original patch  commit
  git format-patch -1
  git send-email --no-chain-reply-to --annotate
 0001-Use-long-variable-type-for-zend_parse_parameters.patch
 
  then I've noticed the warnings (still on parse-param-fix branch)
  create patch  commit
  git fetch --all
  git pull --rebase
  git send-email --no-chain-reply-to --annotate origin/master
 
  I guess that before starting the waring fix patch I should have create a
  new local branch:
  git checkout master
  git pull
  git checkout -b warning-fix origin/master
 
  and then work from there.
 
  Regards,
  Dawid
 
 
  On Wed, 2014-06-18 at 21:28 +0200, Michal Novotny wrote:
   Hi Dawid,
  
  
   thanks for the patch, I'll apply it when I have time to do so.
   However, the patch is not critical as it's in the DPRINTF debug macro
   (for production environment you should disable the DEBUG macro).
  
  
   Also, please make sure you are you the latest git tree (by running git
   pull before writing the patch) as I'm having issues applying some of
   the patches cleanly.
  
  
   Thanks,
   Michal
  
  
   2014-06-18 21:09 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com:
   The previous patch [1] caused compiler warnings after variable
   types
   were changed from int to long and this patch fixes this.
  
   [1]
  
 https://www.redhat.com/archives/libvir-list/2014-June/msg00835.html
   ---
src/libvirt-php.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)
  
   diff --git a/src/libvirt-php.c b/src/libvirt-php.c
   index 224943d..6d6fa81 100644
   --- a/src/libvirt-php.c
   +++ b/src/libvirt-php.c
   @@ -3928,7 +3928,7 @@
   PHP_FUNCTION(libvirt_domain_send_pointer_event)
   RETURN_FALSE;
   }
  
   -   DPRINTF(%s: x = %d, y = %d, clicked = %d, release = %
   d, hostname = %s...\n, PHPFUNC, pos_x, pos_y, clicked,
   release, hostname);
   +   DPRINTF(%s: x = %d, y = %d, clicked = %d, release = %
   d, hostname = %s...\n, PHPFUNC, (int) pos_x, (int) pos_y,
   (int) clicked, release, hostname);
   ret = vnc_send_pointer_event(hostname, tmp, pos_x,
   pos_y, clicked, release);
   if (ret == 0) {
   DPRINTF(%s: Pointer event result is %d\n,
   PHPFUNC, ret);
   @@ -4428,7 +4428,7 @@ PHP_FUNCTION(libvirt_domain_new)
   numNets = i;
  
   snprintf(tmpname, sizeof(tmpname), %s-install,
   name);
   -   DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d
   \n, PHPFUNC, tmpname, memMB, maxmemMB);
   +   DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d
   \n, PHPFUNC, tmpname, (int) memMB, (int) maxmemMB);
   tmp = installation_get_xml(1,
   conn-conn, tmpname, memMB, maxmemMB,
   NULL /* arch */, NULL, vcpus, iso_image,
   vmDisks, numDisks, vmNetworks,
   numNets,
   @@ -6272,7 +6272,7 @@
   PHP_FUNCTION(libvirt_domain_snapshot_delete)
   GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags);
  
   retval = virDomainSnapshotDelete(snapshot-snapshot,
   flags);
   -   DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned
   %d\n, PHPFUNC, snapshot-snapshot, flags, retval);
   +   DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned
   %d\n, PHPFUNC, snapshot-snapshot, (int) flags, retval);
   if (retval == -1) RETURN_FALSE;
   RETURN_TRUE;
}
   @@ -6772,7 +6772,7 @@
   PHP_FUNCTION(libvirt_storagevolume_delete)
   GET_VOLUME_FROM_ARGS(r|l,zvolume,flags);
  
   retval = virStorageVolDelete(volume-volume, flags);
   -   DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d
   \n, PHPFUNC, volume-volume, flags, retval);
   +   DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d
   \n, PHPFUNC, volume-volume, (int) flags, retval);
if (retval != 0) {
   set_error_if_unset(Cannot delete 

Re: [libvirt] [PATCH V2 0/2] libxl: support interface type=network

2014-06-18 Thread Jim Fehlig
Jim Fehlig wrote:
 Patch 1 is a V2 of

   http://www.redhat.com/archives/libvir-list/2014-June/msg00382.html

 Patch 2 is a result of Laine's comments to V1.  It ensures script
 is only allowed with interface types ethernet and bridge.

 Jim Fehlig (2):
   libxl: support interface type=network
   libxl: limit support for specifying an interface script

  src/libxl/libxl_conf.c | 63 
 ++
  1 file changed, 58 insertions(+), 5 deletions(-)
   

Any comments on this version?  Thanks!

Regards,
Jim

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


[libvirt] [PATCH] Initial implementation of new job control api

2014-06-18 Thread Tucker DiNapoli
This is my initial definition of a new internal job control api. I am
working on this as a part of the google summer of code. These patches
contain the core job control api and deal only with managing individual
jobs. I am currently working on writing code using this api to manage
jobs in domains, in such a way that I will be able to replace the
current job control code in qemu and libxl. Ultimately I will use this
to implement job control in the storage driver which is my ultimate
goal for the summer of code.



---
 src/Makefile.am  |   1 +
 src/util/virjobcontrol.c | 574 +++
 src/util/virjobcontrol.h | 342 
 3 files changed, 917 insertions(+)
 create mode 100644 src/util/virjobcontrol.c
 create mode 100644 src/util/virjobcontrol.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 2b9ac61..77de0e7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -118,6 +118,7 @@ UTIL_SOURCES =  
\
util/virinitctl.c util/virinitctl.h \
util/viriptables.c util/viriptables.h   \
util/viriscsi.c util/viriscsi.h \
+   util/virjobcontrol.h util/virjobcontrol.c   \
util/virjson.c util/virjson.h   \
util/virkeycode.c util/virkeycode.h \
util/virkeyfile.c util/virkeyfile.h \
diff --git a/src/util/virjobcontrol.c b/src/util/virjobcontrol.c
new file mode 100644
index 000..04a5246
--- /dev/null
+++ b/src/util/virjobcontrol.c
@@ -0,0 +1,574 @@
+/*
+ * virjobcontrol.c Core implementation of job control
+ *
+ * Copyright (C) 2014 Tucker DiNapoli
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Tucker DiNapoli
+ */
+
+#include config.h
+
+#include virjobcontrol.h
+#include viralloc.h
+#include virtime.h
+#include virlog.h
+VIR_LOG_INIT(virjobcontrol);
+
+VIR_ENUM_IMPL(virJob, 4,
+  none,
+  query,
+  modify,
+  destroy,
+);
+/*
+   No files other then this and virjobcontrol.c should need to
+   have access to the core implmentation of jobs. The code in these
+   files is intended to serve as a base for job control independent of
+   drivers.
+*/
+
+#define LOCK_JOB(job)   \
+virMutexLock(job-lock)
+#define UNLOCK_JOB(job) \
+virMutexUnlock(job-lock)
+#define LOCK_JOB_INFO(job)  \
+virMutexLock(job-info-lock)
+#define UNLOCK_JOB_INFO(job)\
+virMutexUnlock(job-info-lock)
+#define GET_CURRENT_TIME(time)  \
+if (virTimeMillisNow(time)  0) {  \
+return -1;  \
+}
+
+
+#define CHECK_FLAG_ATOMIC(job, flag) (virAtomicIntGet(job-flags)  
VIR_JOB_FLAG_##flag)
+#define CHECK_FLAG(job, flag) (job-flags  VIR_JOB_FLAG_##flag)
+#define SET_FLAG_ATOMIC(job, flag) (virAtomicIntOr(job-flags, 
VIR_JOB_FLAG_##flag))
+#define SET_FLAG(job, flag) (job-flags |= VIR_JOB_FLAG_##flag)
+#define UNSET_FLAG_ATOMIC(job, flag) (virAtomicIntAnd(job-flags, 
(~VIR_JOB_FLAG_##flag)))
+#define UNSET_FLAG(job, flag) (job-flags = (~VIR_JOB_FLAG_##flag))
+#define CLEAR_FLAGS_ATOMIC(job) (virAtomicIntSet(job-flags, 
VIR_JOB_FLAG_NONE))
+#define CLEAR_FLAGS(job) (job-flags = VIR_JOB_FLAG_NONE)
+
+typedef struct _jobHashEntry {
+virJobID id;
+virJobObjPtr job;
+struct _jobHashEntry *next;
+} jobHashEntry;
+
+typedef struct _jobHash {
+jobHashEntry **table;
+size_t size;
+size_t num_entries;
+virRWLock lock;
+} jobHash;
+
+/* Using this incures a cost on every call to a job control function
+   that uses the job control hash table, but it means that no one using
+   job control needs to call an initialization function to use it.
+
+   The other option would be to have a function:
+   virJobControlInit(void)
+   {
+ return virOnce(job_once, jobControlInit);
+   }
+   and require anyone using job control to call it.
+ */
+static struct _jobHash *job_hash; /* Hash table that contians all current jobs 
*/
+static int init_err = 0;
+static virOnceControl job_once = VIR_ONCE_CONTROL_INITIALIZER;
+static void
+jobControlInit(void)
+{
+if (VIR_ALLOC_QUIET(job_hash)  

Re: [libvirt] [php PATCH] Fix compiler warnings after int to long conversion

2014-06-18 Thread Dawid Zamirski
On Wed, 2014-06-18 at 22:50 +0200, Michal Novotny wrote:
 Hi Dawid,
 thanks for the information. Well, please rebase to the latest commit
 of my master branch and resend, thanks a lot!
 
 
 Michal
 

Ok, now I'm puzzled. I've rebased the patch from clean master and the
diff ended up identical to the original one. To double check, I've also
successfully re-applied original mbox file on a fresh git clone, like
this:

git clone git://libvirt.org/libvirt-php
git am ~/\[php_PATCH\]_Fix_compiler_warnings_after_int_to_long_conversion.mbox
Applying: Fix compiler warnings after int to long conversion

It seems that my original patch was correctly generated against remote's
master due to:

git send-email --no-chain-reply-to --annotate origin/master

which means it was made against remote master branch, not my local copy
that was dirty.

At this point, I'm not sure what else I can do on my end.

Sorry for the noise with such a simple patch...

Regards,
Dawid


 
 2014-06-18 22:05 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com:
 Ugh, now I know what happened - my local master branch has
 commits I
 have not sent pull requests for yet so it wasn't in 100% in
 sync with
 remote. Do you want me to resend the patch to the ML?
 
 On Wed, 2014-06-18 at 16:02 -0400, Dawid Zamirski wrote:
  Hi Michal,
 
  I'm pretty sure I did git pull right before sending the
 patch. Here's
  what I did exactly:
 
  On master branch:
 
  git pull
  git checkout -b parse-param-fix origin/master
  created original patch  commit
  git format-patch -1
  git send-email --no-chain-reply-to --annotate
 0001-Use-long-variable-type-for-zend_parse_parameters.patch
 
  then I've noticed the warnings (still on parse-param-fix
 branch)
  create patch  commit
  git fetch --all
  git pull --rebase
  git send-email --no-chain-reply-to --annotate origin/master
 
  I guess that before starting the waring fix patch I should
 have create a
  new local branch:
  git checkout master
  git pull
  git checkout -b warning-fix origin/master
 
  and then work from there.
 
  Regards,
  Dawid
 
 
  On Wed, 2014-06-18 at 21:28 +0200, Michal Novotny wrote:
   Hi Dawid,
  
  
   thanks for the patch, I'll apply it when I have time to do
 so.
   However, the patch is not critical as it's in the DPRINTF
 debug macro
   (for production environment you should disable the DEBUG
 macro).
  
  
   Also, please make sure you are you the latest git tree (by
 running git
   pull before writing the patch) as I'm having issues
 applying some of
   the patches cleanly.
  
  
   Thanks,
   Michal
  
  
   2014-06-18 21:09 GMT+02:00 Dawid Zamirski
 dzamir...@dattobackup.com:
   The previous patch [1] caused compiler warnings
 after variable
   types
   were changed from int to long and this patch fixes
 this.
  
   [1]
  
 https://www.redhat.com/archives/libvir-list/2014-June/msg00835.html
   ---
src/libvirt-php.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)
  
   diff --git a/src/libvirt-php.c b/src/libvirt-php.c
   index 224943d..6d6fa81 100644
   --- a/src/libvirt-php.c
   +++ b/src/libvirt-php.c
   @@ -3928,7 +3928,7 @@
   PHP_FUNCTION(libvirt_domain_send_pointer_event)
   RETURN_FALSE;
   }
  
   -   DPRINTF(%s: x = %d, y = %d, clicked = %d,
 release = %
   d, hostname = %s...\n, PHPFUNC, pos_x, pos_y,
 clicked,
   release, hostname);
   +   DPRINTF(%s: x = %d, y = %d, clicked = %d,
 release = %
   d, hostname = %s...\n, PHPFUNC, (int) pos_x,
 (int) pos_y,
   (int) clicked, release, hostname);
   ret = vnc_send_pointer_event(hostname,
 tmp, pos_x,
   pos_y, clicked, release);
   if (ret == 0) {
   DPRINTF(%s: Pointer event result
 is %d\n,
   PHPFUNC, ret);
   @@ -4428,7 +4428,7 @@
 PHP_FUNCTION(libvirt_domain_new)
   numNets = i;
  
   snprintf(tmpname, sizeof(tmpname), %

[libvirt] [PATCH v3] blockjob: use stable disk string in job event

2014-06-18 Thread Eric Blake
When the block job event was first added, it was for block pull,
where the active layer of the disk remains the same name.  It was
also in a day where we only cared about local files, and so we
always had a canonical absolute file name.  But two things have
changed since then: we now have network disks, where determining
a single absolute string does not really make sense; and we have
two-phase jobs (copy and active commit) where the name of the
active layer changes between the first event (ready, on the old
name) and second (complete, on the pivoted name).

Adam Litke reported that having an unstable string between events
makes life harder for clients.  Furthermore, all of our API that
operate on a particular disk of a domain accept multiple strings:
not only the absolute name of the active layer, but also the
destination device name (such as 'vda').  As this latter name is
stable, even for network sources, it serves as a better string
to supply in block job events.

But backwards-compatibility demands that we should not change the
name handed to users unless they explicitly request it.  Therefore,
this patch adds a new event, BLOCK_JOB_2 (alas, I couldn't think of
any nicer name - but at least Migrate2 and Migrate3 are precedent
for a number suffix).  We must double up on emitting both old-style
and new-style events according to what clients have registered for
(see also how IOError and IOErrorReason emits double events, but
there the difference was a larger struct rather than changed
meaning of one of the struct members).

Unfortunately, adding a new event isn't something that can easily
be broken into pieces, so the commit is rather large.

* include/libvirt/libvirt.h.in (virDomainEventID): Add a new id
for VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2.
(virConnectDomainEventBlockJobCallback): Document new semantics.
* src/conf/domain_event.c (_virDomainEventBlockJob): Rename field,
to ensure we catch all clients.
(virDomainEventBlockJobNew): Add parameter.
(virDomainEventBlockJobDispose)
(virDomainEventBlockJobNewFromObj)
(virDomainEventBlockJobNewFromDom)
(virDomainEventDispatchDefaultFunc): Adjust clients.
(virDomainEventBlockJob2NewFromObj)
(virDomainEventBlockJob2NewFromDom): New functions.
* src/conf/domain_event.h: Add new prototypes.
* src/libvirt_private.syms (domain_event.h): Export new functions.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Generate two
different events.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Likewise.
* src/remote/remote_protocol.x
(remote_domain_event_block_job_2_msg): New struct.
(REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2): New RPC.
* src/remote/remote_driver.c
(remoteDomainBuildEventBlockJob2): New handler.
(remoteEvents): Register new event.
* daemon/remote.c (remoteRelayDomainEventBlockJob2): New handler.
(domainEventCallbacks): Register new event.
* tools/virsh-domain.c (vshEventCallbacks): Likewise.
(vshEventBlockJobPrint): Adjust client.
* src/remote_protocol-structs: Regenerate.

Signed-off-by: Eric Blake ebl...@redhat.com
---

v3: don't do s/path/disk/ on code that is not shared; s/disk/dst/ on
new code that is not shared; touch up libvirt.c docs, actually test
with python bindings

 daemon/remote.c  | 39 +++
 include/libvirt/libvirt.h.in | 19 ---
 src/conf/domain_event.c  | 44 +++-
 src/conf/domain_event.h  | 11 +++
 src/libvirt_private.syms |  2 ++
 src/qemu/qemu_driver.c   |  8 +++-
 src/qemu/qemu_process.c  |  7 +++
 src/remote/remote_driver.c   | 31 +++
 src/remote/remote_protocol.x | 16 +++-
 src/remote_protocol-structs  |  8 
 tools/virsh-domain.c |  7 +--
 11 files changed, 176 insertions(+), 16 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 34c96c9..7199764 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -931,6 +931,44 @@ remoteRelayDomainEventDeviceRemoved(virConnectPtr conn,
 }


+static int
+remoteRelayDomainEventBlockJob2(virConnectPtr conn,
+virDomainPtr dom,
+const char *dst,
+int type,
+int status,
+void *opaque)
+{
+daemonClientEventCallbackPtr callback = opaque;
+remote_domain_event_block_job_2_msg data;
+
+if (callback-callbackID  0 ||
+!remoteRelayDomainEventCheckACL(callback-client, conn, dom))
+return -1;
+
+VIR_DEBUG(Relaying domain block job 2 event %s %d %s %i, %i, callback %d,
+  dom-name, dom-id, dst, type, status, callback-callbackID);
+
+/* build return data */
+memset(data, 0, sizeof(data));
+data.callbackID = callback-callbackID;
+if (VIR_STRDUP(data.dst, dst)  0)
+goto error;
+data.type = type;
+data.status = status;
+make_nonnull_domain(data.dom, dom);
+
+

[libvirt] [python PATCH 1/2] blockjob: support new BLOCK_JOB_2 event

2014-06-18 Thread Eric Blake
Libvirt 1.2.6 is introducing a new block job event that passes disk
information by target device rather than host file name.  At the
python level, we are just a passthrough, so we can reuse all the
existing code and just wire up the new enum value.

* libvirt-override-virConnect.py
(_dispatchDomainEventBlockPullCallback): Rename...
(_dispatchDomainEventBlockJobCallback): ...to this, and make
generic to both events.
* libvirt-override.c
(libvirt_virConnectDomainEventBlockJobCallback): Match naming.
(libvirt_virConnectDomainEventRegisterAny): Allow new registration.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 libvirt-override-virConnect.py |  6 +++---
 libvirt-override.c | 10 +++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/libvirt-override-virConnect.py b/libvirt-override-virConnect.py
index c228eb2..31d71a3 100644
--- a/libvirt-override-virConnect.py
+++ b/libvirt-override-virConnect.py
@@ -113,14 +113,14 @@
authScheme, subject, opaque)
 return 0

-def _dispatchDomainEventBlockPullCallback(self, dom, path, type, status, 
cbData):
-Dispatches events to python user domain blockJob event callbacks
+def _dispatchDomainEventBlockJobCallback(self, dom, disk, type, status, 
cbData):
+Dispatches events to python user domain blockJob/blockJob2 event 
callbacks
 
 try:
 cb = cbData[cb]
 opaque = cbData[opaque]

-cb(self, virDomain(self, _obj=dom), path, type, status, opaque)
+cb(self, virDomain(self, _obj=dom), disk, type, status, opaque)
 return 0
 except AttributeError:
 pass
diff --git a/libvirt-override.c b/libvirt-override.c
index 8fd856b..eb1d5e2 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -6085,7 +6085,7 @@ 
libvirt_virConnectDomainEventGraphicsCallback(virConnectPtr conn ATTRIBUTE_UNUSE
 static int
 libvirt_virConnectDomainEventBlockJobCallback(virConnectPtr conn 
ATTRIBUTE_UNUSED,
   virDomainPtr dom,
-  const char *path,
+  const char *disk,
   int type,
   int status,
   void *opaque)
@@ -6114,9 +6114,9 @@ 
libvirt_virConnectDomainEventBlockJobCallback(virConnectPtr conn ATTRIBUTE_UNUSE

 /* Call the Callback Dispatcher */
 pyobj_ret = PyObject_CallMethod(pyobj_conn,
-
(char*)_dispatchDomainEventBlockPullCallback,
+
(char*)_dispatchDomainEventBlockJobCallback,
 (char*)OsiiO,
-pyobj_dom, path, type, status, 
pyobj_cbData);
+pyobj_dom, disk, type, status, 
pyobj_cbData);

 Py_DECREF(pyobj_cbData);
 Py_DECREF(pyobj_dom);
@@ -6506,6 +6506,7 @@ 
libvirt_virConnectDomainEventDeviceRemovedCallback(virConnectPtr conn ATTRIBUTE_
 }
 #endif /* LIBVIR_CHECK_VERSION(1, 1, 1) */

+
 static PyObject *
 libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject *self,
  PyObject *args)
@@ -6561,6 +6562,9 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED 
PyObject *self,
 cb = 
VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventGenericCallback);
 break;
 case VIR_DOMAIN_EVENT_ID_BLOCK_JOB:
+#if LIBVIR_CHECK_VERSION(1, 2, 6)
+case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2:
+#endif /* LIBVIR_CHECK_VERSION(1, 2, 6) */
 cb = 
VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventBlockJobCallback);
 break;
 case VIR_DOMAIN_EVENT_ID_DISK_CHANGE:
-- 
1.9.3

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


[libvirt] [python PATCH 2/2] event-test: add missing events

2014-06-18 Thread Eric Blake
Update the example to be able to trace all events.

* examples/event-test.py (main): Match full list of domain events.
(myDomainEventIOErrorReasonCallback)
(myDomainEventControlErrorCallback)
(myDomainEventBlockJobCallback, myDomainEventBlockJob2Callback)
(blockJobTypeToString, blockJobStatusToString): New functions.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 examples/event-test.py | 26 ++
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/examples/event-test.py b/examples/event-test.py
index aa3c5a7..cd85de3 100644
--- a/examples/event-test.py
+++ b/examples/event-test.py
@@ -456,6 +456,14 @@ def domDetailToString(event, detail):
 )
 return domEventStrings[event][detail]

+def blockJobTypeToString(type):
+blockJobTypes = ( unknown, Pull, Copy, Commit, ActiveCommit, )
+return blockJobTypes[type]
+
+def blockJobStatusToString(status):
+blockJobStatus = ( Completed, Failed, Canceled, Ready, )
+return blockJobStatus[status]
+
 def myDomainEventCallback1 (conn, dom, event, detail, opaque):
 print(myDomainEventCallback1 EVENT: Domain %s(%s) %s %s % (dom.name(), 
dom.ID(),
  
domEventToString(event),
@@ -477,10 +485,14 @@ def myDomainEventWatchdogCallback(conn, dom, action, 
opaque):

 def myDomainEventIOErrorCallback(conn, dom, srcpath, devalias, action, opaque):
 print(myDomainEventIOErrorCallback: Domain %s(%s) %s %s %d % 
(dom.name(), dom.ID(), srcpath, devalias, action))
-
+def myDomainEventIOErrorReasonCallback(conn, dom, srcpath, devalias, action, 
reason, opaque):
+print(myDomainEventIOErrorReasonCallback: Domain %s(%s) %s %s %d %s % 
(dom.name(), dom.ID(), srcpath, devalias, action, reason))
 def myDomainEventGraphicsCallback(conn, dom, phase, localAddr, remoteAddr, 
authScheme, subject, opaque):
 print(myDomainEventGraphicsCallback: Domain %s(%s) %d %s % (dom.name(), 
dom.ID(), phase, authScheme))
-
+def myDomainEventControlErrorCallback(conn, dom, opaque):
+print(myDomainEventControlErrorCallback: Domain %s(%s) % (dom.name(), 
dom.ID()))
+def myDomainEventBlockJobCallback(conn, dom, disk, type, status, opaque):
+print(myDomainEventBlockJobCallback: Domain %s(%s) %s on disk %s %s % 
(dom.name(), dom.ID(), blockJobTypeToString(type), disk, 
blockJobStatusToString(status)))
 def myDomainEventDiskChangeCallback(conn, dom, oldSrcPath, newSrcPath, 
devAlias, reason, opaque):
 print(myDomainEventDiskChangeCallback: Domain %s(%s) disk change 
oldSrcPath: %s newSrcPath: %s devAlias: %s reason: %s % (
 dom.name(), dom.ID(), oldSrcPath, newSrcPath, devAlias, reason))
@@ -501,6 +513,8 @@ def myDomainEventPMSuspendDiskCallback(conn, dom, reason, 
opaque):
 def myDomainEventDeviceRemovedCallback(conn, dom, dev, opaque):
 print(myDomainEventDeviceRemovedCallback: Domain %s(%s) device removed: 
%s % (
 dom.name(), dom.ID(), dev))
+def myDomainEventBlockJob2Callback(conn, dom, disk, type, status, opaque):
+print(myDomainEventBlockJob2Callback: Domain %s(%s) %s on disk %s %s % 
(dom.name(), dom.ID(), blockJobTypeToString(type), disk, 
blockJobStatusToString(status)))

 ##
 # Network events
@@ -591,14 +605,17 @@ def main():

 vc.registerCloseCallback(myConnectionCloseCallback, None)

-#Add 2 callbacks to prove this works with more than just one
+#Add 2 lifecycle callbacks to prove this works with more than just one
 vc.domainEventRegister(myDomainEventCallback1,None)
 vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, 
myDomainEventCallback2, None)
 vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_REBOOT, 
myDomainEventRebootCallback, None)
 vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_RTC_CHANGE, 
myDomainEventRTCChangeCallback, None)
-vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR, 
myDomainEventIOErrorCallback, None)
 vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_WATCHDOG, 
myDomainEventWatchdogCallback, None)
+vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR, 
myDomainEventIOErrorCallback, None)
 vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_GRAPHICS, 
myDomainEventGraphicsCallback, None)
+vc.domainEventRegisterAny(None, 
libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON, 
myDomainEventIOErrorReasonCallback, None)
+vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_CONTROL_ERROR, 
myDomainEventControlErrorCallback, None)
+vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_BLOCK_JOB, 
myDomainEventBlockJobCallback, None)
 vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_DISK_CHANGE, 
myDomainEventDiskChangeCallback, None)
 vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_TRAY_CHANGE, 
myDomainEventTrayChangeCallback, None)
 

Re: [libvirt] [PATCH libvirt] xen: handle root= in xen-xm configuration files.

2014-06-18 Thread Eric Blake
On 06/18/2014 11:46 AM, Jim Fehlig wrote:


 In addition to extra= xm supported a root= option which was supposed
 to be incorporated into the final command line. Handle that for virsh
 domxml-from-native xen-xm. Tested with the libxl backend.

 Signed-off-by: Ian Campbell ian.campb...@citrix.com
 ---
 v2: extra should default to .
 ---
  .gnulib|2 +-

 --- WTF. I stripped this out of the patch shown below...

  src/xenxs/xen_xm.c |   14 +-
  2 files changed, 14 insertions(+), 2 deletions(-)
   
 
 ACK and pushed; thanks!

I'm now seeing test failures on Fedora 20, non-xen setup:

TEST: xmconfigtest
 1) Xen XM-2-XML Parse  paravirt-old-pvfb ... OK
 2) Xen XM-2-XML Format paravirt-old-pvfb ...
Offset 324
Expect []
Actual [  cmdline/cmdline
  ]

... FAILED
 3) Xen XM-2-XML Parse  paravirt-old-pvfb-vncdisplay  ... OK
 4) Xen XM-2-XML Format paravirt-old-pvfb-vncdisplay  ...
Offset 324
Expect []
Actual [  cmdline/cmdline
  ]

... FAILED
 5) Xen XM-2-XML Parse  paravirt-new-pvfb ... OK

and several others.  Looks like something in the code is outputting an
empty cmdline element when it is not needed.

-- 
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 v3 2/5] blockjob: turn on qemu capability bit for active commit

2014-06-18 Thread Eric Blake
Use the probing functionality added in the last patch to turn on
a capability bit when active commit is present, and gate active
commit on that capability.

While at it, leave a few more bread-crumbs about what blockjob
sync/async means, and enforce that sync cancel is only ever
encountered with blockpull (at this point, RHEL 6.2 is likely
to be the only platform to have sync cancel).

Modifying qemucapabilitiestest is painful; the .replies files would
be so much easier if they had comments correlating which command
generated the given reply.  Maybe I'll fix that up later...

* src/qemu/qemu_capabilities.h (QEMU_CAPS_ACTIVE_COMMIT): New
capability.
(QEMU_CAPS_BLOCKJOB_SYNC): Enhance comment.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Likewise.
(qemuDomainBlockCopy): Likewise.
(qemuDomainBlockCommit): Use the new bit
* src/qemu/qemu_capabilities.c (virQEMUCaps): Name the new bit.
(virQEMUCapsProbeQMPCommands): Set it.
* tests/qemucapabilitiesdata/caps_1.3.1-1.replies: Update.
* tests/qemucapabilitiesdata/caps_1.4.2-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.5.3-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.6.0-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.6.50-1.replies: Likewise.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/qemu/qemu_capabilities.c |  7 +++
 src/qemu/qemu_capabilities.h |  3 +-
 src/qemu/qemu_driver.c   | 16 +++---
 tests/qemucapabilitiesdata/caps_1.3.1-1.replies  | 62 +---
 tests/qemucapabilitiesdata/caps_1.4.2-1.replies  | 62 +---
 tests/qemucapabilitiesdata/caps_1.5.3-1.replies  | 62 +---
 tests/qemucapabilitiesdata/caps_1.6.0-1.replies  | 62 +---
 tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 +---
 8 files changed, 193 insertions(+), 143 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 08c3d04..b96fec1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   usb-kbd, /* 165 */
   host-pci-multidomain,
   msg-timestamp,
+  active-commit,
 );


@@ -2149,6 +2150,12 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps,
 VIR_FORCE_CLOSE(fd);
 }

+/* Probe for active commit of qemu 2.1 (for now, we are choosing
+ * to ignore the fact that qemu 2.0 can also do active commit) */
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_COMMIT) 
+qemuMonitorBlockCommit(mon, bogus, NULL, NULL, 0) == -2)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT);
+
 return 0;
 }

diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d755caa..617986d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -127,7 +127,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_SCSI_BLOCK = 88, /* -device scsi-block */
 QEMU_CAPS_TRANSACTION= 89, /* transaction monitor command */
 QEMU_CAPS_BLOCKJOB_SYNC  = 90, /* RHEL 6.2 block_job_cancel */
-QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1 block-job-cancel */
+QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1/RHEL 6.3 block-job-cancel */
 QEMU_CAPS_SCSI_CD= 92, /* -device scsi-cd */
 QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */
 QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */
@@ -206,6 +206,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */
 QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain  0 in host pci 
address */
 QEMU_CAPS_MSG_TIMESTAMP  = 167, /* -msg timestamp */
+QEMU_CAPS_ACTIVE_COMMIT  = 168, /* block-commit works without 'top' */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ca58d6b..004b63d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15130,8 +15130,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
  * to prevent newly scheduled block jobs from confusing us.  */
 if (mode == BLOCK_JOB_ABORT) {
 if (!async) {
-/* Older qemu that lacked async reporting also lacked
- * active commit, so we can hardcode the event to pull.
+/* Older qemu (RHEL 6.2) that lacked async reporting also
+ * lacked copy and commit, so we can hardcode type_pull.
  * We have to generate two variants of the event. */
 int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
 int status = VIR_DOMAIN_BLOCK_JOB_CANCELED;
@@ -15291,6 +15291,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
 goto endjob;
 }

+/* Ensure that no one backports copy to RHEL 6.2, where cancel
+ * behaved differently */
 if (!(virQEMUCapsGet(priv-qemuCaps, 

[libvirt] [PATCH v3 1/5] blockjob: allow omitted arguments to QMP block-commit

2014-06-18 Thread Eric Blake
We are about to turn on support for active block commit.  Although
qemu 2.0 was the first version to mostly support it, that version
mis-handles 0-length files, and doesn't have anything available for
easy probing.  But qemu 2.1 fixed bugs, and made life simpler by
letting the 'top' argument be optional.  Unless someone begs for
active commit with qemu 2.0, for now we are just going to enable
it only by probing for qemu 2.1 behavior (anyone backporting active
commit can also backport the optional argument behavior).

Although all our actual uses of block-commit supply arguments for
both base and top, we can omit both arguments and use a bogus
device string to trigger an interesting behavior in qemu.  All QMP
commands first do argument validation, failing with GenericError
if a mandatory argument is missing.  Once that passes, the code
in the specific command gets to do further checking, and the qemu
developers made sure that if device is the only supplied argument,
then the block-commit code will look up the device first, with a
failure of DeviceNotFound, before attempting any further argument
validation (most other validations fail with GenericError).  Thus,
the category of error class can reliably be used to decipher
whether the top argument was optional, which in turn implies a
working active commit.  Since we expect our bogus device string to
trigger an error either way, the code is written to return a
distinct return value without spamming the logs.

* src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): Allow NULL for
top and base, for probing purposes.
* src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise, with special return values based on probe.
* tests/qemumonitorjsontest.c (mymain): Enable...
(testQemuMonitorJSONqemuMonitorJSONBlockCommit2): ...a new test.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/qemu/qemu_monitor.c  |  2 +-
 src/qemu/qemu_monitor.h  |  3 +--
 src/qemu/qemu_monitor_json.c | 20 --
 src/qemu/qemu_monitor_json.h |  3 +--
 tests/qemumonitorjsontest.c  | 49 
 5 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 80d7b9d..013a6ad 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3240,7 +3240,7 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char 
*device,
 unsigned long long speed;

 VIR_DEBUG(mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld,
-  mon, device, top, base, bandwidth);
+  mon, device, NULLSTR(top), NULLSTR(base), bandwidth);

 /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
  * limited to LLONG_MAX also for unsigned values */
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31b3204..1c7857e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -663,8 +663,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
const char *top,
const char *base,
unsigned long bandwidth)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
-ATTRIBUTE_NONNULL(4);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

 int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
 const char *cmd,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bedd959..0e4262d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3467,14 +3467,30 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const 
char *device,
 cmd = qemuMonitorJSONMakeCommand(block-commit,
  s:device, device,
  U:speed, speed,
- s:top, top,
- s:base, base,
+ S:top, top,
+ S:base, base,
  NULL);
 if (!cmd)
 return -1;

 if ((ret = qemuMonitorJSONCommand(mon, cmd, reply))  0)
 goto cleanup;
+if (!top  !base) {
+/* Normally we always specify top and base; but omitting them
+ * allows for probing whether qemu is new enough to support
+ * live commit.  */
+if (qemuMonitorJSONHasError(reply, DeviceNotFound)) {
+VIR_DEBUG(block-commit supports active commit);
+ret = -2;
+} else {
+/* This is a false negative for qemu 2.0; but probably not
+ * worth the additional complexity to worry about it */
+VIR_DEBUG(block-commit requires 'top' parameter, 
+  assuming it lacks active commit);
+ret = -3;
+}
+goto cleanup;
+}
 ret = 

[libvirt] [PATCH v3 4/5] blockcommit: track job type in xml

2014-06-18 Thread Eric Blake
A future patch is going to wire up qemu active block commit jobs;
but as they have similar events and are canceled/pivoted in the
same way as block copy jobs, it is easiest to track all bookkeeping
for the commit job by reusing the mirror element.  This patch
adds domain XML to track which job was responsible for creating a
mirroring situation, and adds a job='copy' attribute to all
existing uses of mirror.  Along the way, it also massages the
qemu monitor backend to read the new field in order to generate
the correct type of libvirt job (even though it requires a
future patch to actually cause a qemu event that can be reported
as an active commit).

* docs/schemas/domaincommon.rng: Enhance schema.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (_virDomainDiskDef): Add a field.
* src/conf/domain_conf.c (virDomainBlockJobType): String conversion.
(virDomainDiskDefParseXML): Parse job type.
(virDomainDiskDefFormat): Output job type.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish
active from regular commit.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type.
(qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type
on completion.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New
file.
* tests/qemuxml2xmltest.c (mymain): Drive new test.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 docs/formatdomain.html.in  | 20 ++--
 docs/schemas/domaincommon.rng  | 13 
 src/conf/domain_conf.c | 33 ++-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_driver.c |  3 ++
 src/qemu/qemu_process.c| 18 +++
 .../qemuxml2argv-disk-active-commit.xml| 37 ++
 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |  4 +--
 .../qemuxml2xmlout-disk-mirror-old.xml |  4 +--
 tests/qemuxml2xmltest.c|  1 +
 10 files changed, 113 insertions(+), 21 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 79b85d5..f8a5238 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1878,16 +1878,18 @@
   /dd
   dtcodemirror/code/dt
   dd
-This element is present if the hypervisor has started a block
-copy operation (via the codevirDomainBlockRebase/code
-API), where the mirror location in the codesource/code
-sub-element will eventually have the same contents as the
-source, and with the file format in the
+This element is present if the hypervisor has started a
+long-running block job operation, where the mirror location in
+the codesource/code sub-element will eventually have the
+same contents as the source, and with the file format in the
 sub-element codeformat/code (which might differ from the
-format of the source).  The details of the codesource/code
-sub-element are determined by the codetype/code attribute
-of the mirror, similar to what is done for the
-overall codedisk/code device element. If
+format of the source).  The codejob/code attribute
+mentions which API started the operation (copy for
+the codevirDomainBlockRebase/code API, or active-commit
+for the codevirDomainBlockCommit/code API).  The details
+of the codesource/code sub-element are determined by
+the codetype/code attribute of the mirror, similar to what
+is done for the overall codedisk/code device element. If
 attribute codeready/code is present, then it is known the
 disk is ready to pivot; otherwise, the disk is probably still
 copying.  For now, this element only valid in output; it is
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 33d0308..fe943f9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4186,6 +4186,13 @@
 /attribute
   /optional
   optional
+attribute name='job'
+  choice
+valuecopy/value
+  /choice
+/attribute
+  /optional
+  optional
 interleave
   ref name='diskSourceFile'/
   optional
@@ -4195,6 +4202,12 @@
   /optional
 /group
 group !-- preferred format --
+  attribute name='job'
+choice
+  valuecopy/value
+  valueactive-commit/value
+/choice
+  /attribute
   interleave
 ref name=diskSource/
 optional
diff --git 

[libvirt] [PATCH v3 0/5] Next round of active commit support

2014-06-18 Thread Eric Blake
Browseable at:
http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/gluster
or git checkout git://repo.or.cz/libvirt/ericb.git gluster

Resolves some of the review comments, and in particular FINALLY
turns on capability probing. However, I can't push this to libvirt
until Jeff's patch actually makes it into qemu.git:
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html

and still lacking cleanup of the persistent XML after a pivot

Eric Blake (5):
  blockjob: allow omitted arguments to QMP block-commit
  blockjob: turn on qemu capability bit for active commit
  virsh: expose new active commit controls
  blockcommit: track job type in xml
  blockcommit: turn on active commit

 docs/formatdomain.html.in  | 20 +++
 docs/schemas/domaincommon.rng  | 13 +
 src/conf/domain_conf.c | 33 +++-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_capabilities.c   |  7 +++
 src/qemu/qemu_capabilities.h   |  3 +-
 src/qemu/qemu_driver.c | 59 +++-
 src/qemu/qemu_monitor.c|  2 +-
 src/qemu/qemu_monitor.h|  3 +-
 src/qemu/qemu_monitor_json.c   | 20 ++-
 src/qemu/qemu_monitor_json.h   |  3 +-
 src/qemu/qemu_process.c| 18 ---
 tests/qemucapabilitiesdata/caps_1.3.1-1.replies| 62 --
 tests/qemucapabilitiesdata/caps_1.4.2-1.replies| 62 --
 tests/qemucapabilitiesdata/caps_1.5.3-1.replies| 62 --
 tests/qemucapabilitiesdata/caps_1.6.0-1.replies| 62 --
 tests/qemucapabilitiesdata/caps_1.6.50-1.replies   | 62 --
 tests/qemumonitorjsontest.c| 49 +
 .../qemuxml2argv-disk-active-commit.xml| 37 +
 .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |  4 +-
 .../qemuxml2xmlout-disk-mirror-old.xml |  4 +-
 tests/qemuxml2xmltest.c|  1 +
 tools/virsh-domain.c   | 56 ---
 tools/virsh.pod| 32 +++
 24 files changed, 484 insertions(+), 191 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml

-- 
1.9.3

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


[libvirt] [PATCH v3 3/5] virsh: expose new active commit controls

2014-06-18 Thread Eric Blake
Add knobs to virsh to manage a 2-phase active commit of the top
layer, similar to knobs already present on blockcopy.  While this
code will fail until later patches actually implement the new
knobs in the qemu driver, doing it now proves that the API is
usable and also makes it easier for testing the qemu changes as
they are made.

* tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot,
and --finish options, modeled after blockcopy.
(blockJobImpl): Support --active flag.
* tools/virsh.pod (blockcommit): Document new flags.
(blockjob): Mention 2-phase commit interaction.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 tools/virsh-domain.c | 56 ++--
 tools/virsh.pod  | 32 +-
 2 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d136862..0ae1538 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1492,6 +1492,10 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
 flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW;
 if (vshCommandOptBool(cmd, delete))
 flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE;
+if (vshCommandOptBool(cmd, active) ||
+vshCommandOptBool(cmd, pivot) ||
+vshCommandOptBool(cmd, keep-overlay))
+flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
 ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
 break;
 case VSH_CMD_BLOCK_JOB_COPY:
@@ -1592,13 +1596,18 @@ static const vshCmdOptDef opts_block_commit[] = {
  .type = VSH_OT_DATA,
  .help = N_(path of top file to commit from (default top of chain))
 },
+{.name = active,
+ .type = VSH_OT_BOOL,
+ .help = N_(trigger two-stage active commit of top file)
+},
 {.name = delete,
  .type = VSH_OT_BOOL,
  .help = N_(delete files that were successfully committed)
 },
 {.name = wait,
  .type = VSH_OT_BOOL,
- .help = N_(wait for job to complete)
+ .help = N_(wait for job to complete 
+(with --active, wait for job to sync))
 },
 {.name = verbose,
  .type = VSH_OT_BOOL,
@@ -1606,7 +1615,15 @@ static const vshCmdOptDef opts_block_commit[] = {
 },
 {.name = timeout,
  .type = VSH_OT_INT,
- .help = N_(with --wait, abort if copy exceeds timeout (in seconds))
+ .help = N_(implies --wait, abort if copy exceeds timeout (in seconds))
+},
+{.name = pivot,
+ .type = VSH_OT_BOOL,
+ .help = N_(implies --active --wait, pivot when commit is synced)
+},
+{.name = keep-overlay,
+ .type = VSH_OT_BOOL,
+ .help = N_(implies --active --wait, quit when commit is synced)
 },
 {.name = async,
  .type = VSH_OT_BOOL,
@@ -1620,8 +1637,11 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
 {
 virDomainPtr dom = NULL;
 bool ret = false;
-bool blocking = vshCommandOptBool(cmd, wait);
 bool verbose = vshCommandOptBool(cmd, verbose);
+bool pivot = vshCommandOptBool(cmd, pivot);
+bool finish = vshCommandOptBool(cmd, keep-overlay);
+bool active = vshCommandOptBool(cmd, active) || pivot || finish;
+bool blocking = vshCommandOptBool(cmd, wait);
 int timeout = 0;
 struct sigaction sig_action;
 struct sigaction old_sig_action;
@@ -1632,7 +1652,12 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
 bool quit = false;
 int abort_flags = 0;

+blocking |= vshCommandOptBool(cmd, timeout) || pivot || finish;
 if (blocking) {
+if (pivot  finish) {
+vshError(ctl, %s, _(cannot mix --pivot and --keep-overlay));
+return false;
+}
 if (vshCommandOptTimeoutToMs(ctl, cmd, timeout)  0)
 return false;
 if (vshCommandOptStringReq(ctl, cmd, path, path)  0)
@@ -1650,8 +1675,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
 sigaction(SIGINT, sig_action, old_sig_action);

 GETTIMEOFDAY(start);
-} else if (verbose || vshCommandOptBool(cmd, timeout) ||
-   vshCommandOptBool(cmd, async)) {
+} else if (verbose || vshCommandOptBool(cmd, async)) {
 vshError(ctl, %s, _(missing --wait option));
 return false;
 }
@@ -1683,6 +1707,8 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
 if (verbose)
 vshPrintJobProgress(_(Block Commit),
 info.end - info.cur, info.end);
+if (active  info.cur == info.end)
+break;

 GETTIMEOFDAY(curr);
 if (intCaught || (timeout 
@@ -1709,7 +1735,25 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
 /* printf [100 %] */
 vshPrintJobProgress(_(Block Commit), 0, 1);
 }
-vshPrint(ctl, \n%s, quit ? _(Commit aborted) : _(Commit complete));
+if (!quit  pivot) {
+abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT;
+if (virDomainBlockJobAbort(dom, path, abort_flags)  0) {
+

[libvirt] [PATCH v3 5/5] blockcommit: turn on active commit

2014-06-18 Thread Eric Blake
With this in place, I can (finally!) now do:

virsh blockcommit $dom vda --shallow --verbose --pivot

and watch qemu shorten the backing chain by one, followed by
libvirt automatically updating the dumpxml output, effectively
undoing the work of virsh snapshot-commit --no-metadata --disk-only.
Commit is S much faster than blockpull, when I'm still fairly
close in time to when the temporary qcow2 wrapper file was created
via a snapshot operation!

Still not done: the persistent XML is not updated; which means
stopping and restarting a persistent guest will use the wrong
file and likely fail to boot.

* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live
commit to regular files.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/qemu/qemu_driver.c | 40 +++-
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d19ca18..089668d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15495,9 +15495,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
 unsigned int baseIndex = 0;
 const char *top_parent = NULL;
 bool clean_access = false;
+virStorageSourcePtr mirror = NULL;
+

-/* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
-virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
+/* XXX Add support for COMMIT_DELETE */
+virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+  VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, -1);

 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
@@ -15545,9 +15548,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
  top_parent)))
 goto endjob;

-/* FIXME: qemu 2.0 supports active commit, but as a two-stage
- * process; qemu 2.1 is further improving active commit. We need
- * to start supporting it in libvirt. */
 if (topSource == disk-src) {
 if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -15561,6 +15561,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
disk-dst);
 goto endjob;
 }
+if (disk-mirror) {
+virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+   _(disk '%s' already in active block job),
+   disk-dst);
+goto endjob;
+}
+if (VIR_ALLOC(mirror)  0)
+goto endjob;
 } else if (flags  VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) {
 virReportError(VIR_ERR_INVALID_ARG,
_(active commit requested but '%s' is not active),
@@ -15607,6 +15615,21 @@ qemuDomainBlockCommit(virDomainPtr dom,
VIR_DISK_CHAIN_READ_WRITE)  0))
 goto endjob;

+/* For an active commit, clone enough of the base to act as the mirror */
+if (mirror) {
+/* XXX Support network commits */
+if (baseSource-type != VIR_STORAGE_TYPE_FILE 
+baseSource-type != VIR_STORAGE_TYPE_BLOCK) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(active commit to non-file not supported yet));
+goto endjob;
+}
+mirror-type = baseSource-type;
+if (VIR_STRDUP(mirror-path, baseSource-path)  0)
+goto endjob;
+mirror-format = baseSource-format;
+}
+
 /* Start the commit operation.  Pass the user's original spelling,
  * if any, through to qemu, since qemu may behave differently
  * depending on whether the input was specified as relative or
@@ -15619,6 +15642,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
  bandwidth);
 qemuDomainObjExitMonitor(driver, vm);

+if (ret == 0  mirror) {
+disk-mirror = mirror;
+mirror = NULL;
+disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
+}
+
  endjob:
 if (ret  0  clean_access) {
 /* Revert access to read-only, if possible.  */
@@ -15629,6 +15658,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
   top_parent,
   VIR_DISK_CHAIN_READ_ONLY);
 }
+virStorageSourceFree(mirror);
 if (!qemuDomainObjEndJob(driver, vm))
 vm = NULL;

-- 
1.9.3

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