Re: [libvirt] [PATCH RFC v2 1/5] conf: add net device prefix to capabilities

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 09:40:33PM +, Joao Martins wrote:
> In the reverted commit d2e5538b1, the libxl driver was changed to copy
> interface names autogenerated by libxl to the corresponding network def
> in the domain's virDomainDef object. The copied name is freed when the
> domain transitions to the shutoff state. But when migrating a domain,
> the autogenerated name is included in the XML sent to the destination
> host.  It is possible an interface with the same name already exists on
> the destination host, causing migration to fail.
> 
> This patch defines a new capability for setting the network device
> prefix that will be used in the driver. Valid prefixes are
> VIR_NET_GENERATED_PREFIX or the one announced by the driver.
> 
> Signed-off-by: Joao Martins 
> ---
> Changes since v1:
>  - free netprefix in virCapabilitiesDispose()
> ---
>  src/conf/capabilities.c  | 22 ++
>  src/conf/capabilities.h  |  4 
>  src/libvirt_private.syms |  1 +
>  3 files changed, 27 insertions(+)

ACK


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

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

Re: [libvirt] [PATCH v3] systemd: Modernize machine naming

[Martin Kletzander]

On Thu, Feb 04, 2016 at 01:37:29PM +0100, Pavel Hrdina wrote:

On Thu, Feb 04, 2016 at 11:40:36AM +0100, Martin Kletzander wrote:

[...]


@@ -290,12 +306,15 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
 tmp++;

 if (STRNEQ(tmp, name) &&
+STRNEQ_NULLABLE(tmp, machinename) &&
 STRNEQ(tmp, partname) &&
-STRNEQ(tmp, scopename)) {
+STRNEQ(tmp, scopename_old) &&
+STRNEQ_NULLABLE(tmp, scopename_new)) {
 VIR_DEBUG("Name '%s' for controller '%s' does not match "
-  "'%s', '%s' or '%s'",
+  "'%s', '%s', '%s', '%s' or '%s'",
   tmp, virCgroupControllerTypeToString(i),
-  name, partname, scopename);
+  name, machinename, partname,
+  scopename_old, scopename_new);


One thing, the machinename and scopename_new can by NULL and we should use
NULLSTR().



Good catch!


 goto cleanup;
 }
 }


ACK with that fix.



Thanks, I'll push it in a while with the following diff squashed in
(just for the sake of completeness):

diff --git i/src/util/vircgroup.c w/src/util/vircgroup.c
index e62793e31e2f..2f54cf29943f 100644
--- i/src/util/vircgroup.c
+++ w/src/util/vircgroup.c
@@ -313,8 +313,8 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
VIR_DEBUG("Name '%s' for controller '%s' does not match "
  "'%s', '%s', '%s', '%s' or '%s'",
  tmp, virCgroupControllerTypeToString(i),
-  name, machinename, partname,
-  scopename_old, scopename_new);
+  name, NULLSTR(machinename), partname,
+  scopename_old, NULLSTR(scopename_new));
goto cleanup;
}
}
--

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3] systemd: Modernize machine naming

[Martin Kletzander]

So, systemd-machined has this philosophy that machine names are like
hostnames and hence should follow the same rules.  But we always allowed
international characters in domain names.  Thus we need to modify the
machine name we are passing to systemd.

In order to change some machine names that we will be passing to systemd,
we also need to call TerminateMachine at the end of a lifetime of a
domain.  Even for domains that were started with older libvirt.  That
can be achieved thanks to virSystemdGetMachineNameByPID().  And because
we can change machine names, we can get rid of the inconsistent and
pointless escaping of domain names when creating machine names.

So this patch modifies the naming in the following way.  It creates the
name as -- where invalid hostname characters are
stripped out of the name and if the resulting name is longer, it
truncates it to 64 characters.  That way we can start domains we
couldn't start before.  Well, at least on systemd.

To make it work all together, the machineName (which is needed only with
systemd) is saved in domain's private data.  That way the generation is
moved to the driver and we don't need to pass various unnecessary
arguments to cgroup functions.

The only thing this complicates a bit is the scope generation when
validating a cgroup where we must check both old and new naming, so a
slight modification was needed there.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1282846

Signed-off-by: Martin Kletzander 
---
 src/lxc/lxc_cgroup.c| 13 +++--
 src/lxc/lxc_domain.h|  1 +
 src/lxc/lxc_process.c   | 20 +
 src/qemu/qemu_cgroup.c  | 28 +-
 src/qemu/qemu_cgroup.h  |  2 +-
 src/qemu/qemu_domain.h  |  1 +
 src/qemu/qemu_process.c |  4 +--
 src/util/vircgroup.c| 59 --
 src/util/vircgroup.h| 12 
 src/util/virsystemd.c   | 75 +
 src/util/virsystemd.h   | 13 -
 tests/virsystemdtest.c  | 67 +--
 12 files changed, 191 insertions(+), 104 deletions(-)

diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index ad254e4934fc..31489466cfbf 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -29,6 +29,7 @@
 #include "viralloc.h"
 #include "vircgroup.h"
 #include "virstring.h"
+#include "virsystemd.h"

 #define VIR_FROM_THIS VIR_FROM_LXC

@@ -483,6 +484,13 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
 int *nicindexes)
 {
 virCgroupPtr cgroup = NULL;
+char *machineName = virSystemdMakeMachineName("lxc",
+  def->id,
+  def->name,
+  true);
+
+if (!machineName)
+goto cleanup;

 if (def->resource->partition[0] != '/') {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -491,9 +499,8 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
 goto cleanup;
 }

-if (virCgroupNewMachine(def->name,
+if (virCgroupNewMachine(machineName,
 "lxc",
-true,
 def->uuid,
 NULL,
 initpid,
@@ -517,6 +524,8 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def,
 }

  cleanup:
+VIR_FREE(machineName);
+
 return cgroup;
 }

diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
index 2119c7899007..39c6e7def9ce 100644
--- a/src/lxc/lxc_domain.h
+++ b/src/lxc/lxc_domain.h
@@ -64,6 +64,7 @@ struct _virLXCDomainObjPrivate {
 pid_t initpid;

 virCgroupPtr cgroup;
+char *machineName;
 };

 extern virDomainXMLNamespace virLXCDriverDomainXMLNamespace;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index f7e2b810b74b..9b3981eab20d 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -233,8 +233,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
  * properly. See https://bugs.freedesktop.org/show_bug.cgi?id=68370 for
  * the bug we are working around here.
  */
-virSystemdTerminateMachine(vm->def->name, "lxc", true);
-
+virCgroupTerminateMachine(priv->machineName);

 /* The "release" hook cleans up additional resources */
 if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
@@ -1494,8 +1493,9 @@ int virLXCProcessStart(virConnectPtr conn,
  * point so lets detect that first, since it gives us a
  * more reliable way to kill everything off if something
  * goes wrong from here onwards ... */
-if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid,
-  -1, >cgroup) < 0)
+if (virCgroupNewDetectMachine(vm->def->name, "lxc",
+  vm->def->id, true,
+  vm->pid, -1, >cgroup) < 0)
 goto cleanup;

 if (!priv->cgroup) 

Re: [libvirt] [PATCH RFC v2 3/5] conf: add caps to virDomainDefFormat*

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 09:40:35PM +, Joao Martins wrote:
> And use the newly added caps->host.netprefix (if it exists) for
> interface names that match the autogenerated target names.
> 
> Signed-off-by: Joao Martins 
> ---
>  src/bhyve/bhyve_driver.c|  9 -
>  src/conf/domain_conf.c  | 28 ++--
>  src/conf/domain_conf.h  |  3 +++
>  src/conf/snapshot_conf.c|  2 +-
>  src/esx/esx_driver.c|  5 +++--
>  src/libxl/libxl_domain.c|  2 +-
>  src/libxl/libxl_driver.c|  9 ++---
>  src/libxl/libxl_migration.c |  2 +-
>  src/lxc/lxc_driver.c|  4 +++-
>  src/lxc/lxc_process.c   | 12 ++--
>  src/network/bridge_driver.c |  4 ++--
>  src/openvz/openvz_driver.c  |  5 +++--
>  src/phyp/phyp_driver.c  |  2 +-
>  src/qemu/qemu_domain.c  |  2 +-
>  src/test/test_driver.c  |  6 --
>  src/uml/uml_driver.c|  2 +-
>  src/vbox/vbox_common.c  |  3 ++-
>  src/vmware/vmware_driver.c  |  5 +++--
>  src/vz/vz_driver.c  |  3 ++-
>  src/xen/xen_driver.c|  4 ++--
>  src/xenapi/xenapi_driver.c  |  5 +++--
>  tests/lxcconf2xmltest.c |  2 +-
>  tests/openvzutilstest.c |  2 +-
>  tests/qemuargv2xmltest.c|  2 +-
>  tests/qemuhotplugtest.c |  3 ++-
>  tests/sexpr2xmltest.c   |  2 +-
>  tests/testutils.c   |  2 +-
>  tests/vmx2xmltest.c |  3 ++-
>  tests/xlconfigtest.c|  2 +-
>  tests/xmconfigtest.c|  2 +-
>  30 files changed, 85 insertions(+), 52 deletions(-)

ACK


> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 0141009..6ae2e1b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2733,11 +2733,13 @@ void virDomainIOThreadSchedDelId(virDomainDefPtr def, 
> unsigned int iothread_id);
>  unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
>  
>  char *virDomainDefFormat(virDomainDefPtr def,
> + virCapsPtr caps,
>   unsigned int flags);
>  char *virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
>   virDomainObjPtr obj,
>   unsigned int flags);

Surprised we don't have caps added to this one too, since it
will call virDomainDefFormat.

>  int virDomainDefFormatInternal(virDomainDefPtr def,
> +   virCapsPtr caps,
> unsigned int flags,
> virBufferPtr buf);
>  
> @@ -2748,6 +2750,7 @@ int virDomainDiskSourceFormat(virBufferPtr buf,
>  
>  int virDomainNetDefFormat(virBufferPtr buf,
>virDomainNetDefPtr def,
> +  char *prefix,
>unsigned int flags);
>  
>  typedef enum {
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index ea511ec..ffa1bf2 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -702,7 +702,7 @@ char *virDomainSnapshotDefFormat(const char *domain_uuid,
>  virBufferAddLit(, "\n");
>  }
>  if (def->dom) {
> -if (virDomainDefFormatInternal(def->dom, flags, ) < 0) {
> +if (virDomainDefFormatInternal(def->dom, NULL, flags, ) < 0) {
>  virBufferFreeAndReset();
>  return NULL;
>  }

Yeah, we'll need to pass caps in here too eventually.

ACK regardless

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 RFC v2 5/5] libxl: set net device prefix

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 09:40:37PM +, Joao Martins wrote:
> Use the newly added virCapabilitiesSetNetPrefix to set
> the network prefix for the driver. This in return will
> be use by NetDefFormat() and NetDefParseXML() routines
> to free any interface name that start with the registered
> prefix.
> 
> Acked-by: Daniel P. Berrange 
> Signed-off-by: Joao Martins 
> ---
>  src/libxl/libxl_conf.c | 3 +++
>  src/libxl/libxl_conf.h | 4 
>  2 files changed, 7 insertions(+)

ACK

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

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

Re: [libvirt] [PATCH RFC v2 4/5] conf: add caps to virDomainSaveConfig

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 09:40:36PM +, Joao Martins wrote:
> virDomainSaveConfig calls virDomainDefFormat which was setting the caps
> to NULL, thus keeping the old behaviour (i.e. not looking at
> netprefix). This patch adds the virCapsPtr to the function and allows
> the configuration to be saved and skipping interface names that were
> registered with virCapabilitiesSetNetPrefix().
> 
> Signed-off-by: Joao Martins 
> ---
>  src/bhyve/bhyve_driver.c  |  2 +-
>  src/conf/domain_conf.c|  5 +++--
>  src/conf/domain_conf.h|  1 +
>  src/libxl/libxl_driver.c  | 15 ---
>  src/lxc/lxc_driver.c  | 20 +++-
>  src/qemu/qemu_blockjob.c  |  1 +
>  src/qemu/qemu_driver.c| 47 
> ++-
>  src/qemu/qemu_migration.c |  3 ++-
>  src/qemu/qemu_process.c   |  5 +++--
>  src/uml/uml_driver.c  |  2 +-
>  10 files changed, 57 insertions(+), 44 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH v3] systemd: Modernize machine naming

[Pavel Hrdina]

On Thu, Feb 04, 2016 at 11:40:36AM +0100, Martin Kletzander wrote:

[...]

> @@ -290,12 +306,15 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
>  tmp++;
> 
>  if (STRNEQ(tmp, name) &&
> +STRNEQ_NULLABLE(tmp, machinename) &&
>  STRNEQ(tmp, partname) &&
> -STRNEQ(tmp, scopename)) {
> +STRNEQ(tmp, scopename_old) &&
> +STRNEQ_NULLABLE(tmp, scopename_new)) {
>  VIR_DEBUG("Name '%s' for controller '%s' does not match "
> -  "'%s', '%s' or '%s'",
> +  "'%s', '%s', '%s', '%s' or '%s'",
>tmp, virCgroupControllerTypeToString(i),
> -  name, partname, scopename);
> +  name, machinename, partname,
> +  scopename_old, scopename_new);

One thing, the machinename and scopename_new can by NULL and we should use
NULLSTR().

>  goto cleanup;
>  }
>  }

ACK with that fix.

Pavel

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

Re: [libvirt] [PATCH] libxl: support feature

[Joao Martins]

On 02/04/2016 01:41 AM, Jim Fehlig wrote:
> On 02/03/2016 02:20 PM, Joao Martins wrote:
>>
>> On 02/03/2016 04:12 AM, Jim Fehlig wrote:
>>> Even though the libxl driver advertises  in capabilities,
>>> it is ignored when set in domXML. Enable hap in the
>>> libxl_domain_create_info struct when  is specified in domXML.
>>>
>>> The xm/xl <-> domXML parser/formatter already supports hap but
>>> lacked a test with hap enabled.  Add a hap test.
> [...]
>> FWIW looks good and also:
>>
>> Tested-by: Joao Martins 
> 
> Thanks for reviewing/testing the patch! But after playing with the patch a bit
> more, I'm considering dropping it :-/.
> 
> Before the patch,  was ignored by the libxl driver, which left the 'hap'
> field of libxl_domain_create_info struct unset (= LIBXL__DEFBOOL_DEFAULT). For
> HVM guests, libxl will enable hap when libxl_domain_create_info.hap ==
> LIBXL__DEFBOOL_DEFAULT. And using hap is good, since using shadow page table 
> is
> less efficient. The downside is there is no way to turn hap off.
> 
> After the patch it is possible to turn hap on and off via presence or absence 
> of
> . However, there is a *lot* of existing config without  that 
> currently
> reaps the benefits of hap nonetheless. After patch, that config would require
> adding , else shadow page table will be used. (Note that in xm and xl
> config, hap is enabled in the absence of 'hap='.)
> 
> I'm starting to question whether this feature should even be exposed. I think
> the only use case is debugging. E.g. turn hap off if suspected hardware bug.
> Otherwise you would certainly want the hypervisor to use hap if it is able to 
> do
> so. What do you think?
> 
Hm, good point. There is another field for VIR_TRISTATE which represents when
it's absent (VIR_TRISTATE_SWITCH_ABSENT). So perhaps we would leave the default
(= LIBXL_DEFBOOL_DEFAULT) if it's absent, and only set hap if the user
explicitly switches on/off? I am not sure if the usecase is just debug, but
given there are certain performance differences compared to shadow paging
(depending on the HVM guest workload) it might be nice to allow the
administrator to choose it.

Joao

> Regards,
> Jim
> 
> 
> --
> 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

Re: [libvirt] [PATCH v3 4/4] logical: Display thin lv's found in a libvirt managed volume group

[Ján Tomko]

On Wed, Feb 03, 2016 at 03:51:30PM -0500, John Ferlan wrote:
> 
> 
> On 02/03/2016 03:23 AM, Ján Tomko wrote:
> > On Tue, Feb 02, 2016 at 11:14:01AM -0500, John Ferlan wrote:
> >>
> >>
> >> On 02/02/2016 08:30 AM, Pavel Hrdina wrote:
> >>> On Tue, Feb 02, 2016 at 09:11:51AM +0100, Ján Tomko wrote:
>  A thin pool is another layer on top of some of the LVs in the VG.
>  I think it deserves a separate pool type.
> >>>
> >>> I must agree with Jan, and also already wrote the same for v2 patch.  The 
> >>> thin
> >>> pool and thin LV shouldn't be mixed with normal logical pool even they 
> >>> share the
> >>> same VG.
> >>>
> >>
> >> If LVM allows it, then who are we to say it cannot or shouldn't work or
> >> how it should be configured?
> > 
> > Even if they are mixed in the same VG, we could show the "thick" LVs
> > in the type='logical' pool and the thin ones in type='thin'.
> > 
> 
> We could if our logic to create/build the pool was overhauled or perhaps
> support was added to have two volume groups use the same source device.
> 
> Since it's possible to place a thin_pool in the same vg as a snapshot
> and a "thick" lv, should we be in the business of splitting hairs and
> trying to define how things should be viewed?
> 

We already are. E.g. in the 'fs' pool, we ignore fifos and subdirectories.

> I don't see the benefit behind requiring the user to decide whether to
> have a libvirt pool view either thin lv's or non-thin lv's of their vg
> or requiring their vg to essentially be one thin-pool in order to
> support thin lv's, when we could support whatever configuration they've
> already developed.
> 

The thin pool would view the thin volumes in a specific thin pool, not
all thin pools in that VG.

> If more work is done in LV pools - are we going to create new pool types
> for other lvm segtypes ("mirror", "stripe", "raid", etc.)? In order to
> display/fetch interesting or specific things about them?
> 

AFAIK none of these create another pool on top of the VG.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH RFC v2 2/5] conf: add prefix in virDomainNetDefParseXML

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 09:40:34PM +, Joao Martins wrote:
> And use the newly added caps->host.netprefix for free interface
> names that match the autogenerated target names.
> 
> Signed-off-by: Joao Martins 
> ---
>  src/conf/domain_conf.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

ACK


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

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

[libvirt] [PATCH] conf: add caps to virDomainSnapshotDefFormat

[Joao Martins]

The virDomainSnapshotDefFormat calls into virDomainDefFormat,
so should be providing a non-NULL virCapsPtr instance. On the
qemu driver we change qemuDomainSnapshotWriteMetadata to also
include caps since it calls virDomainSnapshotDefFormat.

Signed-off-by: Joao Martins 
---
 src/conf/snapshot_conf.c  |  3 ++-
 src/conf/snapshot_conf.h  |  1 +
 src/esx/esx_driver.c  |  2 +-
 src/qemu/qemu_domain.c|  5 +++--
 src/qemu/qemu_domain.h|  1 +
 src/qemu/qemu_driver.c| 16 ++--
 src/test/test_driver.c|  3 ++-
 src/vbox/vbox_common.c|  4 ++--
 tests/domainsnapshotxml2xmltest.c |  2 +-
 9 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ffa1bf2..1eda7c2 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -659,6 +659,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
 
 char *virDomainSnapshotDefFormat(const char *domain_uuid,
  virDomainSnapshotDefPtr def,
+ virCapsPtr caps,
  unsigned int flags,
  int internal)
 {
@@ -702,7 +703,7 @@ char *virDomainSnapshotDefFormat(const char *domain_uuid,
 virBufferAddLit(, "\n");
 }
 if (def->dom) {
-if (virDomainDefFormatInternal(def->dom, NULL, flags, ) < 0) {
+if (virDomainDefFormatInternal(def->dom, caps, flags, ) < 0) {
 virBufferFreeAndReset();
 return NULL;
 }
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 4f0d096..fcf7a1e 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -113,6 +113,7 @@ virDomainSnapshotDefPtr 
virDomainSnapshotDefParseNode(xmlDocPtr xml,
 void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def);
 char *virDomainSnapshotDefFormat(const char *domain_uuid,
  virDomainSnapshotDefPtr def,
+ virCapsPtr caps,
  unsigned int flags,
  int internal);
 int virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapshot,
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index ce5f799..00d0e0a 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4292,7 +4292,7 @@ esxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
 
 virUUIDFormat(snapshot->domain->uuid, uuid_string);
 
-xml = virDomainSnapshotDefFormat(uuid_string, ,
+xml = virDomainSnapshotDefFormat(uuid_string, , priv->caps,
  virDomainDefFormatConvertXMLFlags(flags),
  0);
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a70b866..0b02c68 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2605,6 +2605,7 @@ qemuFindQemuImgBinary(virQEMUDriverPtr driver)
 int
 qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
 virDomainSnapshotObjPtr snapshot,
+virCapsPtr caps,
 char *snapshotDir)
 {
 char *newxml = NULL;
@@ -2615,7 +2616,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
 
 virUUIDFormat(vm->def->uuid, uuidstr);
 newxml = virDomainSnapshotDefFormat(
-uuidstr, snapshot->def,
+uuidstr, snapshot->def, caps,
 virDomainDefFormatConvertXMLFlags(QEMU_DOMAIN_FORMAT_LIVE_FLAGS),
 1);
 if (newxml == NULL)
@@ -2775,7 +2776,7 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
  snap->def->parent);
 } else {
 parentsnap->def->current = true;
-if (qemuDomainSnapshotWriteMetadata(vm, parentsnap,
+if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, 
driver->caps,
 cfg->snapshotDir) < 0) {
 VIR_WARN("failed to set parent snapshot '%s' as current",
  snap->def->parent);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6a8cf70..7cabf10 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -375,6 +375,7 @@ const char *qemuFindQemuImgBinary(virQEMUDriverPtr driver);
 
 int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
 virDomainSnapshotObjPtr snapshot,
+virCapsPtr caps,
 char *snapshotDir);
 
 int qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d623831..2e283a9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14908,6 +14908,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
 if (update_current) {
 

Re: [libvirt] [PATCH] libxl: support feature

[Joao Martins]

On 02/04/2016 06:22 PM, Jim Fehlig wrote:
> On 02/04/2016 05:54 AM, Joao Martins wrote:
>>
>> On 02/04/2016 01:41 AM, Jim Fehlig wrote:
>>> On 02/03/2016 02:20 PM, Joao Martins wrote:
 On 02/03/2016 04:12 AM, Jim Fehlig wrote:
> Even though the libxl driver advertises  in capabilities,
> it is ignored when set in domXML. Enable hap in the
> libxl_domain_create_info struct when  is specified in domXML.
>
> The xm/xl <-> domXML parser/formatter already supports hap but
> lacked a test with hap enabled.  Add a hap test.
>>> [...]
 FWIW looks good and also:

 Tested-by: Joao Martins 
>>> Thanks for reviewing/testing the patch! But after playing with the patch a 
>>> bit
>>> more, I'm considering dropping it :-/.
>>>
>>> Before the patch,  was ignored by the libxl driver, which left the 
>>> 'hap'
>>> field of libxl_domain_create_info struct unset (= LIBXL__DEFBOOL_DEFAULT). 
>>> For
>>> HVM guests, libxl will enable hap when libxl_domain_create_info.hap ==
>>> LIBXL__DEFBOOL_DEFAULT. And using hap is good, since using shadow page 
>>> table is
>>> less efficient. The downside is there is no way to turn hap off.
>>>
>>> After the patch it is possible to turn hap on and off via presence or 
>>> absence of
>>> . However, there is a *lot* of existing config without  that 
>>> currently
>>> reaps the benefits of hap nonetheless. After patch, that config would 
>>> require
>>> adding , else shadow page table will be used. (Note that in xm and xl
>>> config, hap is enabled in the absence of 'hap='.)
>>>
>>> I'm starting to question whether this feature should even be exposed. I 
>>> think
>>> the only use case is debugging. E.g. turn hap off if suspected hardware bug.
>>> Otherwise you would certainly want the hypervisor to use hap if it is able 
>>> to do
>>> so. What do you think?
>>>
>> Hm, good point. There is another field for VIR_TRISTATE which represents when
>> it's absent (VIR_TRISTATE_SWITCH_ABSENT). So perhaps we would leave the 
>> default
>> (= LIBXL_DEFBOOL_DEFAULT) if it's absent, and only set hap if the user
>> explicitly switches on/off?
> 
> But IIUC, absent == off. I.e., there is no way to turn hap off except the
> absence of .
> 
Ah, right. I was double checking and there's really no way to have it.

> One option might be to extend  with an 'enabled=yes|no' attribute. Then 
> the
> absence of  means hypervisor default.  without the attribute would 
> be
> the same as  for backwards compatibility. And of course 
>  enabled='no'/> disables hap.
> 
And it sounds that it could be applicable for other features too: e.g. pae
(instead of having both pae and nonpae?)

> Note that capabilities currently advertises hap as
> 
>   
> 
> If folks agree to this option, the default should be changed to 'on'.
> 
>>  I am not sure if the usecase is just debug, but
>> given there are certain performance differences compared to shadow paging
>> (depending on the HVM guest workload) it might be nice to allow the
>> administrator to choose it.
> 
> Agreed. I think the above option accomplishes that.
Yeap, I wonder if the absence of a "default" in the capabilities could turn into
"hypervisor/driver default" since you can explicitly choose "on" or "off"
already. Or perhaps it doesn't make a lot of sense to do that.

Joao

> 
> Regards,
> Jim
> 
> --
> 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

Re: [libvirt] [PATCH v3 7/8] daemon: add connection close rpc

[Nikolay Shirokovskiy]

On 04.02.2016 18:33, Daniel P. Berrange wrote:
> On Mon, Feb 01, 2016 at 03:40:20PM +0300, Nikolay Shirokovskiy wrote:
>> remoteConnectUnregisterCloseCallback is not quite good.
>> if it is given a callback function different from that
>> was registered before then local part will fail silently. On
>> the other hand we can not gracefully handle this fail
>> as the remote part is already unregistered.
> 
> We could sanity check the callback before unregistering the
> remote part. Or you could do the local unregister first since
> if the remote part then fails, it is harmless - we'll see the
> close event frm the server still, but we won't dispatch it.

I'd prefer sanity check. The second option makes reregistering
impossible (however i doubt this is really a usecase). Will resend
soon.

> 
>> There are a lot of options to fix it. I think of totally
>> removing the callback argument from unregistering. What's
>> the use of it?
> 
> We can't remove it since that would change ABI.
> 

Yes, I forgot about this.

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

[libvirt] [PATCH] bhyve: fix build

[Roman Bogorodskiy]

Fix build fail introduced as a side effect of commit d239a54.

Pushed under the build breaker rule.
---
 src/bhyve/bhyve_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 337a29e..f486f86 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -488,7 +488,7 @@ bhyveDomainGetOSType(virDomainPtr dom)
 static char *
 bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
 {
-bhyveConnPtr privconn = conn->privateData;
+bhyveConnPtr privconn = domain->conn->privateData;
 virDomainObjPtr vm;
 virCapsPtr caps = NULL;
 char *ret = NULL;
-- 
2.4.6

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

[libvirt] [PATCH 1/4] vol: Add new elements to _virStorageVolSource for thin lv

[John Ferlan]

A thin lv doesn't have any extents defined - rather it uses a concept of
a "thin-pool" in order to describe it's source. In order to allow that to
be displayed properly in a future patch, add a new 'thin_pool' name that
can be used by a future patch to store the name of the source thin_pool
name used by a thin logical volume. A thin_pool has a specific size, so
add a thin_capacity field to store that.

For now, these are "output only" fields. That is - if a thin lv is found
in a volume group, we can display the thinpool data.  The output will
appear in a vol-dumpxml (similar to how extents are handled) as:

  

  20971520

  

The new tests add the "bones" in order to at some point in a future patch
add the ability to parse the input XML with a  and generate a
thinpool within the volume group.  Although the storagevolxml2xmltest
added valid input (so that storagevolschematest can validate the format),
the output will not be listed, thus the storagevolxml2xmlout does not
have the source data (similar to vol-logical.xml).

Signed-off-by: John Ferlan 
---
 docs/schemas/storagevol.rng | 14 ++
 src/conf/storage_conf.c | 11 +++
 src/conf/storage_conf.h |  3 +++
 tests/storagevolxml2xmlin/vol-logical-thin.xml  | 20 
 tests/storagevolxml2xmlout/vol-logical-thin.xml | 17 +
 tests/storagevolxml2xmltest.c   |  1 +
 6 files changed, 66 insertions(+)
 create mode 100644 tests/storagevolxml2xmlin/vol-logical-thin.xml
 create mode 100644 tests/storagevolxml2xmlout/vol-logical-thin.xml

diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 7450547..25e2db7 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -144,6 +144,9 @@
   
 
   
+  
+
+
 
   
 
@@ -172,6 +175,17 @@
 
   
 
+  
+
+  
+
+  
+  
+
+  
+
+  
+
   
 
   none
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3657dfd..8eef399 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -322,6 +322,7 @@ virStorageVolDefFree(virStorageVolDefPtr def)
 for (i = 0; i < def->source.nextent; i++)
 VIR_FREE(def->source.extents[i].path);
 VIR_FREE(def->source.extents);
+VIR_FREE(def->source.thin_pool);
 
 virStorageSourceClear(>target);
 VIR_FREE(def);
@@ -1655,6 +1656,16 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
 virBufferAddLit(, "\n");
 }
 
+if (def->source.thin_pool) {
+virBufferEscapeString(, "\n",
+  def->source.thin_pool);
+virBufferAdjustIndent(, 2);
+virBufferAsprintf(, "%llu\n",
+  def->source.thin_capacity);
+virBufferAdjustIndent(, -2);
+virBufferAddLit(, "\n");
+}
+
 virBufferAdjustIndent(, -2);
 virBufferAddLit(, "\n");
 
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 31b45be..f19cb59 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -55,6 +55,9 @@ struct _virStorageVolSource {
 
 int partType; /* virStorageVolTypeDisk, only used by disk
* backend for partition type creation */
+
+char *thin_pool;   /* For a thin volume, the thin-pool and capacity */
+unsigned long long thin_capacity; /* bytes */
 };
 
 
diff --git a/tests/storagevolxml2xmlin/vol-logical-thin.xml 
b/tests/storagevolxml2xmlin/vol-logical-thin.xml
new file mode 100644
index 000..7d97627
--- /dev/null
+++ b/tests/storagevolxml2xmlin/vol-logical-thin.xml
@@ -0,0 +1,20 @@
+
+  thinmint1
+  r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0
+  
+
+  20971520
+
+  
+  2080374784
+  2080374784
+  
+/dev/HostVG/thinmints
+
+  0660
+  0
+  6
+  system_u:object_r:fixed_disk_device_t:s0
+
+  
+
diff --git a/tests/storagevolxml2xmlout/vol-logical-thin.xml 
b/tests/storagevolxml2xmlout/vol-logical-thin.xml
new file mode 100644
index 000..198d3ea
--- /dev/null
+++ b/tests/storagevolxml2xmlout/vol-logical-thin.xml
@@ -0,0 +1,17 @@
+
+  thinmint1
+  r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0
+  
+  
+  2080374784
+  2080374784
+  
+/dev/HostVG/thinmints
+
+  0660
+  0
+  6
+  system_u:object_r:fixed_disk_device_t:s0
+
+  
+
diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c
index 148d1e6..197d308 100644
--- a/tests/storagevolxml2xmltest.c
+++ b/tests/storagevolxml2xmltest.c
@@ -107,6 +107,7 @@ mymain(void)
 DO_TEST("pool-dir", "vol-qcow2-nobacking");
 DO_TEST("pool-disk", "vol-partition");
 DO_TEST("pool-logical", "vol-logical");
+DO_TEST("pool-logical", "vol-logical-thin");
 DO_TEST("pool-logical", "vol-logical-backing");
 DO_TEST("pool-sheepdog", "vol-sheepdog");
 DO_TEST("pool-gluster", "vol-gluster-dir");
-- 
2.5.0

--

[libvirt] [PATCH 0/4] Display thin lv's from a logical vg

[John Ferlan]

While I know not the preferred mechanism for Jan, these patches
provide a means to display thin lv data from a volume group including
the source thin-pool name and thin-pool capacity. Since a thin-pool
and a thin lv are just members of the vg, it just feels natural to
display whatever is in the volume group because a logical pool
comprises all the volumes within the vg.

Even if not accepted, some concepts can be used as a bases for a
single thin-pool to libvirt pool relationship.

There are no changes to virsh vol-list --details or virsh vol-info
to display the thin-pool capacity. If one self adds all the capacity
values in the vol-list --details display, they will determine that the
sum is larger than the virsh pool-info output for the volume group.
Although that's even true prior to these patches since the thin data
is not displayed in the vol-list output, but is accounted for in the
pool-info output, thus the sum of the vol-list capacity details are less.


John Ferlan (4):
  vol: Add new elements to _virStorageVolSource for thin lv
  logical: Add capability to get the thin pool name
  logical: Add thin-pool look-aside list
  logical: Display thin lv's found in a libvirt managed volume group

 docs/formatstorage.html.in  |   9 ++-
 docs/schemas/storagevol.rng |  14 
 src/conf/storage_conf.c |  11 +++
 src/conf/storage_conf.h |  15 
 src/storage/storage_backend_logical.c   | 100 +++-
 tests/storagevolxml2xmlin/vol-logical-thin.xml  |  20 +
 tests/storagevolxml2xmlout/vol-logical-thin.xml |  17 
 tests/storagevolxml2xmltest.c   |   1 +
 8 files changed, 166 insertions(+), 21 deletions(-)
 create mode 100644 tests/storagevolxml2xmlin/vol-logical-thin.xml
 create mode 100644 tests/storagevolxml2xmlout/vol-logical-thin.xml

-- 
2.5.0

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

[libvirt] [PATCH 3/4] logical: Add thin-pool look-aside list

[John Ferlan]

During processing of the extents found in a pool, we have historically
ignored the thin-pool which means any thin lv found in the pool would
also be ignored. This can start to change now - we can save aside the
name and capacity of any thin-pool's found so that we can use that when
we find a thin lv and fill in the thin-pool capacity value on output.

The result will end up being the following for a vol-dumpxml:

  

  20971520

  

instead of an empty   pair.

An upcoming patch will allow a thin lv to be seen

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.h   | 12 +
 src/storage/storage_backend_logical.c | 51 +--
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index f19cb59..a9a1288 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -47,6 +47,18 @@ struct _virStorageVolSourceExtent {
 unsigned long long end;
 };
 
+/*
+ * How to represent thin-pool's for a logical volume. Used by the
+ * logical parsing code
+ */
+typedef struct _virStorageVolSourceThinPool virStorageVolSourceThinPool;
+typedef virStorageVolSourceThinPool *virStorageVolSourceThinPoolPtr;
+struct _virStorageVolSourceThinPool {
+char *name;
+unsigned long long capacity;
+};
+
+
 typedef struct _virStorageVolSource virStorageVolSource;
 typedef virStorageVolSource *virStorageVolSourcePtr;
 struct _virStorageVolSource {
diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index 3044853..d7990e2 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -72,6 +72,8 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
 struct virStorageBackendLogicalPoolVolData {
 virStoragePoolObjPtr pool;
 virStorageVolDefPtr vol;
+size_t nthinpools;
+virStorageVolSourceThinPoolPtr thinpools;
 };
 
 static int
@@ -221,12 +223,38 @@ virStorageBackendLogicalMakeVol(char **const groups,
 return 0;
 
 /*
- * Skip thin pools(t). These show up in normal lvs output
+ * Save the thin pools(t). These show up in normal lvs output
  * but do not have a corresponding /dev/$vg/$lv device that
  * is created by udev. This breaks assumptions in later code.
+ * The thin pools are the "capacity container" for all the thin
+ * lv's found in the pool. A thin lv provides a virtual size upon
+ * creation and can appear to over subscribe the pool capacity.
+ * Although usually thin pools are listed before thin lv's in the
+ * output, we'll just wait until all volumes are processed and
+ * then match the name saved here with thin_pool name we saved
+ * earlier to get the capacity value of thin-pool into the volume.
+ * It's expected that there is more than 1 thin lv per thin-pool.
+ * There can be more than 1 thin-pool per volume group.
  */
-if (attrs[0] == 't')
+if (attrs[0] == 't') {
+virStorageVolSourceThinPool thin;
+
+memset(, 0, sizeof(thin));
+if (VIR_STRDUP(thin.name, groups[0]) < 0)
+goto cleanup;
+if (virStrToLong_ull(groups[8], NULL, 10, ) < 0) {
+VIR_FREE(thin.name);
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("malformed thin pool capacity value"));
+goto cleanup;
+}
+if (VIR_APPEND_ELEMENT(data->thinpools, data->nthinpools, thin) < 0) {
+VIR_FREE(thin.name);
+goto cleanup;
+}
+
 return 0;
+}
 
 /* See if we're only looking for a specific volume */
 if (data->vol != NULL) {
@@ -389,7 +417,10 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
 struct virStorageBackendLogicalPoolVolData cbdata = {
 .pool = pool,
 .vol = vol,
+.nthinpools = 0,
+.thinpools = NULL,
 };
+size_t i, j;
 
 cmd = virCommandNewArgList(LVS,
"--separator", "#",
@@ -410,9 +441,25 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
"lvs") < 0)
 goto cleanup;
 
+/* If we find some thin-pools during processing, let's see if we
+ * need information from them for any thin lv's in the pool
+ */
+for (i = 0; i < cbdata.nthinpools; i++) {
+for (j = 0; j < pool->volumes.count; j++) {
+if (STREQ_NULLABLE(pool->volumes.objs[j]->source.thin_pool,
+   cbdata.thinpools[i].name)) {
+pool->volumes.objs[j]->source.thin_capacity =
+cbdata.thinpools[i].capacity;
+}
+}
+}
+
 ret = 0;
  cleanup:
 virCommandFree(cmd);
+for (i = 0; i < cbdata.nthinpools; i++)
+VIR_FREE(cbdata.thinpools[i].name);
+VIR_FREE(cbdata.thinpools);
 return ret;
 }
 
-- 
2.5.0

--
libvir-list mailing list

[libvirt] [PATCH 2/4] logical: Add capability to get the thin pool name

[John Ferlan]

Since a future patch will start displaying thin logical volumes for a
logical volume group thin-pool, add the ability to fetch the name
of the thin-pool.

This is accomplished by adding 'pool_lv' to the list of regex's and
when a "thin" lv segtype (groups[4]) is found, we can store the
thin-pool name.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_logical.c | 27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index ba26223..3044853 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -67,6 +67,7 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
 #define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED "striped"
 #define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_MIRROR  "mirror"
 #define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_RAID"raid"
+#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_THIN"thin"
 
 struct virStorageBackendLogicalPoolVolData {
 virStoragePoolObjPtr pool;
@@ -291,6 +292,11 @@ virStorageBackendLogicalMakeVol(char **const groups,
VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0)
 goto cleanup;
 
+if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_THIN)) {
+if (VIR_STRDUP(vol->source.thin_pool, groups[10]) < 0)
+goto cleanup;
+}
+
 if (virStrToLong_ull(groups[8], NULL, 10, >target.allocation) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume allocation value"));
@@ -323,9 +329,10 @@ virStorageBackendLogicalMakeVol(char **const groups,
 #define VIR_STORAGE_VOL_LOGICAL_VG_EXTENT_SIZE_REGEX "([0-9]+)#"
 #define VIR_STORAGE_VOL_LOGICAL_SIZE_REGEX "([0-9]+)#"
 #define VIR_STORAGE_VOL_LOGICAL_LV_ATTR_REGEX "(\\S+)#"
+#define VIR_STORAGE_VOL_LOGICAL_POOL_LV_REGEX "(\\S*)#"
 #define VIR_STORAGE_VOL_LOGICAL_SUFFIX_REGEX "?\\s*$"
 
-#define VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT 10
+#define VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT 11
 #define VIR_STORAGE_VOL_LOGICAL_REGEX \
VIR_STORAGE_VOL_LOGICAL_PREFIX_REGEX \
VIR_STORAGE_VOL_LOGICAL_LV_NAME_REGEX \
@@ -338,6 +345,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
VIR_STORAGE_VOL_LOGICAL_VG_EXTENT_SIZE_REGEX \
VIR_STORAGE_VOL_LOGICAL_SIZE_REGEX \
VIR_STORAGE_VOL_LOGICAL_LV_ATTR_REGEX \
+   VIR_STORAGE_VOL_LOGICAL_POOL_LV_REGEX \
VIR_STORAGE_VOL_LOGICAL_SUFFIX_REGEX
 
 static int
@@ -346,14 +354,15 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
 {
 /*
  * # lvs --separator # --noheadings --units b --unbuffered --nosuffix 
--options \
- * 
"lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr"
 VGNAME
+ * 
"lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr,pool_lv"
 VGNAME
  *
- * 
RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#linear#1#5234491392#33554432#5234491392#-wi-ao
- * 
SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#linear#1#1040187392#33554432#1040187392#-wi-ao
- * 
Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#linear#1#1073741824#33554432#1073741824#owi-a-
- * 
Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#linear#1#2181038080#33554432#2181038080#-wi-a-
- * 
Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#linear#1#1040187392#33554432#1040187392#swi-a-
- * 
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#/dev/sdc1(10240),/dev/sdd1(0)#striped#2#42949672960#4194304#-wi-a-
+ * 
RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#linear#1#5234491392#33554432#5234491392#-wi-ao#
+ * 
SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#linear#1#1040187392#33554432#1040187392#-wi-ao#
+ * 
Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#linear#1#1073741824#33554432#1073741824#owi-a-#
+ * 
Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#linear#1#2181038080#33554432#2181038080#-wi-a-#
+ * 
Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#linear#1#1040187392#33554432#1040187392#swi-a-#
+ * 
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#/dev/sdc1(10240),/dev/sdd1(0)#striped#2#42949672960#4194304#-wi-a-#
+ * 
test_thin##3DvSec-w7Bq-Ukep-Ut62-dOCO-ODE3-vx8Okc##thin#0#1#41943040#4194304#41943040#41943040#Vwi-a-tz--#thinpool_lv_test_thin
  *
  * Pull out name, origin, & uuid, device, device extent start #,
  * segment size, extent size, size, attrs
@@ -389,7 +398,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
"--unbuffered",
"--nosuffix",
"--options",
-   

[libvirt] [PATCH 4/4] logical: Display thin lv's found in a libvirt managed volume group

[John Ferlan]

Modify the regex for the 'devices' (a/k/a 'extents') from "(\\S+)"
(e.g., 1 or more) to "(\\S*)" (e.g., zero or more).

Then for any "thin" lv's found, mark the volume as a sparse volume so
that the volume wipe algorithm doesn't work.

Since a "thin" segtype has no devices, this will result in any "thin"
lv part of some thin-pool within a volume group used as a libvirt pool
to be displayed as a possible volume to use.

NB:
Based on a proposal authored by Joe Harvell ,
but with much intervening rework, the resulting patch is changed from
the original concept. About all that remains is changing the regex and
checking for NULL/empty field during parse.

Signed-off-by: John Ferlan 
---
 docs/formatstorage.html.in|  9 +++--
 src/storage/storage_backend_logical.c | 22 ++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 4965a4c..2f4662c 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -566,8 +566,13 @@
 Since 0.4.1
   source
   Provides information about the underlying storage allocation
-of the volume. This may not be available for some pool types.
-Since 0.4.1
+of the volume. This is available logical pool types to display
+details of logical volume extent information
+(Since 0.4.1)
+or the name of the name of the thin-pool used by the thin
+logical volume
+(Since 1.3.2).
+
   target
   Provides information about the representation of the volume
 on the local host. Since 0.4.1
diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index d7990e2..15fa228 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -91,9 +91,13 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr 
vol,
 unsigned long long offset, size, length;
 virStorageVolSourceExtent extent;
 
+/* If the devices field is NULL or empty, then there's nothing to do */
+if (!groups[3] || !*groups[3])
+return 0;
+
 memset(, 0, sizeof(extent));
 
-/* Assume 1 extent (the regex for 'devices' is "(\\S+)") and only
+/* Assume 1 extent (since we checked for NULL or empty above) and only
  * check the 'stripes' field if we have a striped, mirror, or one of
  * the raid (raid1, raid4, raid5*, raid6*, or raid10) segtypes in which
  * case the stripes field will denote the number of lv's within the
@@ -286,13 +290,15 @@ virStorageBackendLogicalMakeVol(char **const groups,
 goto cleanup;
 }
 
-/* Mark the (s) sparse/snapshot lv, e.g. the lv created using
- * the --virtualsize/-V option. We've already ignored the (t)hin
- * pool definition. In the manner libvirt defines these, the
- * thin pool is hidden to the lvs output, except as the name
- * in brackets [] described for the groups[1] (backingStore).
+/* Mark the (s) sparse/snapshot or the (V) thin/thin-pool member lv,
+ * e.g. the lv created using the --virtualsize/-V option to ensure
+ * the volume wipe algorithm doesn't overwrite sparse/thin volumes.
+ * We've already ignored the (t)hin pool definition. In the manner
+ * libvirt defines these, the thin pool is hidden to the lvs output,
+ * except as the name in brackets [] described for the groups[1]
+ * (backingStore).
  */
-if (attrs[0] == 's')
+if (attrs[0] == 's' || attrs[0] == 'V')
 vol->target.sparse = true;
 
 /* Skips the backingStore of lv created with "--virtualsize",
@@ -350,7 +356,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
 #define VIR_STORAGE_VOL_LOGICAL_LV_NAME_REGEX "(\\S+)#"
 #define VIR_STORAGE_VOL_LOGICAL_ORIGIN_REGEX "(\\S*)#"
 #define VIR_STORAGE_VOL_LOGICAL_UUID_REGEX "(\\S+)#"
-#define VIR_STORAGE_VOL_LOGICAL_DEVICES_REGEX "(\\S+)#"
+#define VIR_STORAGE_VOL_LOGICAL_DEVICES_REGEX "(\\S*)#"
 #define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_REGEX "(\\S+)#"
 #define VIR_STORAGE_VOL_LOGICAL_STRIPES_REGEX "([0-9]+)#"
 #define VIR_STORAGE_VOL_LOGICAL_SEG_SIZE_REGEX "(\\S+)#"
-- 
2.5.0

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

[libvirt] [PATCH 11/11] conf: Move and optimize disk target duplicity checking

[Peter Krempa]

Move the logic from virDomainDiskDefDstDuplicates into
virDomainDiskDefCheckDuplicateInfo so that we don't have to loop
multiple times through the array of disks. Since the original function
was called in qemuBuildDriveDevStr, it was actually called for every
single disk which was quite wasteful.

Additionally the target uniqueness check needed to be duplicated in
the disk hotplug case, since the disk was inserted into the domain
definition after the device string was formatted and thus
virDomainDiskDefDstDuplicates didn't do anything in that case.
---
 src/conf/domain_conf.c   | 44 +---
 src/conf/domain_conf.h   |  1 -
 src/libvirt_private.syms |  1 -
 src/qemu/qemu_command.c  |  3 ---
 src/qemu/qemu_hotplug.c  |  6 --
 5 files changed, 13 insertions(+), 42 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8a7a989..c1dffc4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12988,31 +12988,6 @@ virDomainDiskControllerMatch(int controller_type, int 
disk_bus)
 return false;
 }

-/* Return true if there's a duplicate disk[]->dst name for the same bus */
-bool
-virDomainDiskDefDstDuplicates(virDomainDefPtr def)
-{
-size_t i, j;
-
-/* optimization */
-if (def->ndisks <= 1)
-return false;
-
-for (i = 1; i < def->ndisks; i++) {
-for (j = 0; j < i; j++) {
-if (STREQ(def->disks[i]->dst, def->disks[j]->dst)) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("target '%s' duplicated for disk sources "
- "'%s' and '%s'"),
-   def->disks[i]->dst,
-   NULLSTR(virDomainDiskGetSource(def->disks[i])),
-   NULLSTR(virDomainDiskGetSource(def->disks[j])));
-return true;
-}
-}
-}
-return false;
-}

 int
 virDomainDiskIndexByAddress(virDomainDefPtr def,
@@ -23924,6 +23899,15 @@ int
 virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a,
virDomainDiskDefPtr b)
 {
+if (STREQ(a->dst, b->dst)) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("target '%s' duplicated for disk sources '%s' and 
'%s'"),
+   a->dst,
+   NULLSTR(virDomainDiskGetSource(a)),
+   NULLSTR(virDomainDiskGetSource(b)));
+return -1;
+}
+
 if (a->wwn && b->wwn && STREQ(a->wwn, b->wwn)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Disks '%s' and '%s' have identical WWN"),
@@ -23949,12 +23933,10 @@ virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr 
def)
 size_t j;

 for (i = 0; i < def->ndisks; i++) {
-if (def->disks[i]->wwn || def->disks[i]->serial) {
-for (j = i + 1; j < def->ndisks; j++) {
-if (virDomainDiskDefCheckDuplicateInfo(def->disks[i],
-   def->disks[j]) < 0)
-return -1;
-}
+for (j = i + 1; j < def->ndisks; j++) {
+if (virDomainDiskDefCheckDuplicateInfo(def->disks[i],
+   def->disks[j]) < 0)
+return -1;
 }
 }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cbf01bf..8a95b20 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2763,7 +2763,6 @@ int virDomainDefCompatibleDevice(virDomainDefPtr def,

 void virDomainRNGDefFree(virDomainRNGDefPtr def);

-bool virDomainDiskDefDstDuplicates(virDomainDefPtr def);
 int virDomainDiskIndexByAddress(virDomainDefPtr def,
 virDevicePCIAddressPtr pci_controller,
 unsigned int bus, unsigned int target,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bf25473..11de1f8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -257,7 +257,6 @@ virDomainDiskCacheTypeFromString;
 virDomainDiskCacheTypeToString;
 virDomainDiskDefAssignAddress;
 virDomainDiskDefCheckDuplicateInfo;
-virDomainDiskDefDstDuplicates;
 virDomainDiskDefForeachPath;
 virDomainDiskDefFree;
 virDomainDiskDefNew;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8943270..d7f19f3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4228,9 +4228,6 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
 const char *contAlias;
 int controllerModel;

-if (virDomainDiskDefDstDuplicates(def))
-goto error;
-
 if (qemuCheckDiskConfig(disk) < 0)
 goto error;

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 18a5a12..8771780 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -794,12 +794,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 case VIR_DOMAIN_DISK_DEVICE_DISK:
 case 

[libvirt] [PATCH 05/11] qemu: hotplug: Extract common code to qemuDomainAttachDeviceDiskLive

[Peter Krempa]

Target uniqueness check was duplicated in all of the three workers
called from it. Extract it to the parent.
---
 src/qemu/qemu_hotplug.c | 35 +--
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index fa83c6e..2e5cf64 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -315,7 +315,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
  virDomainObjPtr vm,
  virDomainDiskDefPtr disk)
 {
-size_t i;
 int ret = -1;
 const char* type = virDomainDiskBusTypeToString(disk->bus);
 qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -338,14 +337,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 goto cleanup;
 }

-for (i = 0; i < vm->def->ndisks; i++) {
-if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("target %s already exists"), disk->dst);
-goto cleanup;
-}
-}
-
 if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
 goto cleanup;

@@ -577,14 +568,6 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
 int ret = -1;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

-for (i = 0; i < vm->def->ndisks; i++) {
-if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("target %s already exists"), disk->dst);
-goto cleanup;
-}
-}
-
 if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
 goto cleanup;

@@ -688,21 +671,12 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
  virDomainDiskDefPtr disk)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
-size_t i;
 int ret = -1;
 char *drivestr = NULL;
 char *devstr = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 const char *src = virDomainDiskGetSource(disk);

-for (i = 0; i < vm->def->ndisks; i++) {
-if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("target %s already exists"), disk->dst);
-goto cleanup;
-}
-}
-
 if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
 goto cleanup;

@@ -770,6 +744,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
 {
+size_t i;
 virDomainDiskDefPtr disk = dev->data.disk;
 virDomainDiskDefPtr orig_disk = NULL;
 int ret = -1;
@@ -818,6 +793,14 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,

 case VIR_DOMAIN_DISK_DEVICE_DISK:
 case VIR_DOMAIN_DISK_DEVICE_LUN:
+for (i = 0; i < vm->def->ndisks; i++) {
+if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("target %s already exists"), disk->dst);
+goto cleanup;
+}
+}
+
 switch ((virDomainDiskBus) disk->bus) {
 case VIR_DOMAIN_DISK_BUS_USB:
 if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
-- 
2.6.2

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

[libvirt] [PATCH 03/11] qemu: hotplug: Break up if/else statement into switch

[Peter Krempa]

---
 src/qemu/qemu_hotplug.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0101063..43f686c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -818,19 +818,31 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,

 case VIR_DOMAIN_DISK_DEVICE_DISK:
 case VIR_DOMAIN_DISK_DEVICE_LUN:
-if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+switch ((virDomainDiskBus) disk->bus) {
+case VIR_DOMAIN_DISK_BUS_USB:
 if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("disk device='lun' is not supported for usb 
bus"));
 break;
 }
-ret = qemuDomainAttachUSBMassStorageDevice(conn, driver, vm,
-   disk);
-} else if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ret = qemuDomainAttachUSBMassStorageDevice(conn, driver, vm, disk);
+break;
+
+case VIR_DOMAIN_DISK_BUS_VIRTIO:
 ret = qemuDomainAttachVirtioDiskDevice(conn, driver, vm, disk);
-} else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+break;
+
+case VIR_DOMAIN_DISK_BUS_SCSI:
 ret = qemuDomainAttachSCSIDisk(conn, driver, vm, disk);
-} else {
+break;
+
+case VIR_DOMAIN_DISK_BUS_IDE:
+case VIR_DOMAIN_DISK_BUS_FDC:
+case VIR_DOMAIN_DISK_BUS_XEN:
+case VIR_DOMAIN_DISK_BUS_UML:
+case VIR_DOMAIN_DISK_BUS_SATA:
+case VIR_DOMAIN_DISK_BUS_SD:
+case VIR_DOMAIN_DISK_BUS_LAST:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("disk bus '%s' cannot be hotplugged."),
virDomainDiskBusTypeToString(disk->bus));
-- 
2.6.2

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

[libvirt] [PATCH 08/11] qemu: process: Reorder operations on early VM startup

[Peter Krempa]

Retrieval of the driver capabilities as well as emulator capabilities
does not require the complete qemuProcessStop to be executed on
failure.
---
 src/qemu/qemu_process.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7b09ba7..0f617da 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4431,7 +4431,14 @@ qemuProcessInit(virQEMUDriverPtr driver,
 }

 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
-goto stop;
+goto cleanup;
+
+VIR_DEBUG("Determining emulator version");
+virObjectUnref(priv->qemuCaps);
+if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
+  vm->def->emulator,
+  vm->def->os.machine)))
+goto cleanup;

 /* Some things, paths, ... are generated here and we want them to persist.
  * Fill them in prior to setting the domain def as transient. */
@@ -4461,13 +4468,6 @@ qemuProcessInit(virQEMUDriverPtr driver,
  VIR_HOOK_SUBOP_BEGIN) < 0)
 goto stop;

-VIR_DEBUG("Determining emulator version");
-virObjectUnref(priv->qemuCaps);
-if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
-  vm->def->emulator,
-  vm->def->os.machine)))
-goto stop;
-
 ret = 0;

  cleanup:
-- 
2.6.2

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

[libvirt] [PATCH 06/11] conf: Extract code that checks disk serial/wwn conflict

[Peter Krempa]

Put it into a separate function that can be called on two disk def
pointers.
---
 src/conf/domain_conf.c | 42 --
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 55e7ed9..2ef6609 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23920,6 +23920,28 @@ virDomainDefNeedsPlacementAdvice(virDomainDefPtr def)
 }


+static int
+virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a,
+   virDomainDiskDefPtr b)
+{
+if (a->wwn && b->wwn && STREQ(a->wwn, b->wwn)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Disks '%s' and '%s' have identical WWN"),
+   a->dst, b->dst);
+return -1;
+}
+
+if (a->serial && b->serial && STREQ(a->serial, b->serial)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Disks '%s' and '%s' have identical serial"),
+   a->dst, b->dst);
+return -1;
+}
+
+return 0;
+}
+
+
 int
 virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def)
 {
@@ -23929,25 +23951,9 @@ virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def)
 for (i = 0; i < def->ndisks; i++) {
 if (def->disks[i]->wwn || def->disks[i]->serial) {
 for (j = i + 1; j < def->ndisks; j++) {
-if (def->disks[i]->wwn &&
-STREQ_NULLABLE(def->disks[i]->wwn,
-   def->disks[j]->wwn)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Disks '%s' and '%s' have identical WWN"),
-   def->disks[i]->dst,
-   def->disks[j]->dst);
+if (virDomainDiskDefCheckDuplicateInfo(def->disks[i],
+   def->disks[j]) < 0)
 return -1;
-}
-
-if (def->disks[i]->serial &&
-STREQ_NULLABLE(def->disks[i]->serial,
-   def->disks[j]->serial)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Disks '%s' and '%s' have identical 
serial"),
-   def->disks[i]->dst,
-   def->disks[j]->dst);
-return -1;
-}
 }
 }
 }
-- 
2.6.2

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

[libvirt] [PATCH 04/11] qemu: hotplug: Use more common 'cleanup' label in qemuDomainAttachDeviceDiskLive

[Peter Krempa]

---
 src/qemu/qemu_hotplug.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 43f686c..fa83c6e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -779,20 +779,20 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported driver name '%s' for disk '%s'"),
virDomainDiskGetDriver(disk), src);
-goto end;
+goto cleanup;
 }

 if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
-goto end;
+goto cleanup;

 if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
-goto end;
+goto cleanup;

 if (qemuSetUnprivSGIO(dev) < 0)
-goto end;
+goto cleanup;

 if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
-goto end;
+goto cleanup;

 switch ((virDomainDiskDevice) disk->device)  {
 case VIR_DOMAIN_DISK_DEVICE_CDROM:
@@ -805,12 +805,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
  "by libvirt"),
virDomainDiskBusTypeToString(disk->bus),
disk->dst);
-goto end;
+goto cleanup;
 }

 if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
disk->src, false) < 0)
-goto end;
+goto cleanup;

 disk->src = NULL;
 ret = 0;
@@ -853,7 +853,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 break;
 }

- end:
+ cleanup:
 if (ret != 0)
 ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
 return ret;
-- 
2.6.2

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

[libvirt] [PATCH 02/11] qemu: hotplug: Remove unnecessary variable

[Peter Krempa]

---
 src/qemu/qemu_hotplug.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b456fed..0101063 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -773,13 +773,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 virDomainDiskDefPtr disk = dev->data.disk;
 virDomainDiskDefPtr orig_disk = NULL;
 int ret = -1;
-const char *driverName = virDomainDiskGetDriver(disk);
 const char *src = virDomainDiskGetSource(disk);

-if (driverName && STRNEQ(driverName, "qemu")) {
+if (STRNEQ_NULLABLE(virDomainDiskGetDriver(disk), "qemu")) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported driver name '%s' for disk '%s'"),
-   driverName, src);
+   virDomainDiskGetDriver(disk), src);
 goto end;
 }

-- 
2.6.2

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

Re: [libvirt] [PATCH RFC v2 3/5] conf: add caps to virDomainDefFormat*

[Joao Martins]

On 02/04/2016 11:14 AM, Daniel P. Berrange wrote:
> On Wed, Feb 03, 2016 at 09:40:35PM +, Joao Martins wrote:
>> And use the newly added caps->host.netprefix (if it exists) for
>> interface names that match the autogenerated target names.
>>
>> Signed-off-by: Joao Martins 
>> ---
>>  src/bhyve/bhyve_driver.c|  9 -
>>  src/conf/domain_conf.c  | 28 ++--
>>  src/conf/domain_conf.h  |  3 +++
>>  src/conf/snapshot_conf.c|  2 +-
>>  src/esx/esx_driver.c|  5 +++--
>>  src/libxl/libxl_domain.c|  2 +-
>>  src/libxl/libxl_driver.c|  9 ++---
>>  src/libxl/libxl_migration.c |  2 +-
>>  src/lxc/lxc_driver.c|  4 +++-
>>  src/lxc/lxc_process.c   | 12 ++--
>>  src/network/bridge_driver.c |  4 ++--
>>  src/openvz/openvz_driver.c  |  5 +++--
>>  src/phyp/phyp_driver.c  |  2 +-
>>  src/qemu/qemu_domain.c  |  2 +-
>>  src/test/test_driver.c  |  6 --
>>  src/uml/uml_driver.c|  2 +-
>>  src/vbox/vbox_common.c  |  3 ++-
>>  src/vmware/vmware_driver.c  |  5 +++--
>>  src/vz/vz_driver.c  |  3 ++-
>>  src/xen/xen_driver.c|  4 ++--
>>  src/xenapi/xenapi_driver.c  |  5 +++--
>>  tests/lxcconf2xmltest.c |  2 +-
>>  tests/openvzutilstest.c |  2 +-
>>  tests/qemuargv2xmltest.c|  2 +-
>>  tests/qemuhotplugtest.c |  3 ++-
>>  tests/sexpr2xmltest.c   |  2 +-
>>  tests/testutils.c   |  2 +-
>>  tests/vmx2xmltest.c |  3 ++-
>>  tests/xlconfigtest.c|  2 +-
>>  tests/xmconfigtest.c|  2 +-
>>  30 files changed, 85 insertions(+), 52 deletions(-)
> 
> ACK
> 
> 
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 0141009..6ae2e1b 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2733,11 +2733,13 @@ void virDomainIOThreadSchedDelId(virDomainDefPtr 
>> def, unsigned int iothread_id);
>>  unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
>>  
>>  char *virDomainDefFormat(virDomainDefPtr def,
>> + virCapsPtr caps,
>>   unsigned int flags);
>>  char *virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
>>   virDomainObjPtr obj,
>>   unsigned int flags);
> 
> Surprised we don't have caps added to this one too, since it
> will call virDomainDefFormat.
Ah sorry, this one slipped. It would need changing virDomainSaveStatus to
include caps (and propagating that change to qemu, lxc, libxl and bhyve too) so
perhaps in a separate patch?

> 
>>  int virDomainDefFormatInternal(virDomainDefPtr def,
>> +   virCapsPtr caps,
>> unsigned int flags,
>> virBufferPtr buf);
>>  
>> @@ -2748,6 +2750,7 @@ int virDomainDiskSourceFormat(virBufferPtr buf,
>>  
>>  int virDomainNetDefFormat(virBufferPtr buf,
>>virDomainNetDefPtr def,
>> +  char *prefix,
>>unsigned int flags);
>>  
>>  typedef enum {
>> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
>> index ea511ec..ffa1bf2 100644
>> --- a/src/conf/snapshot_conf.c
>> +++ b/src/conf/snapshot_conf.c
>> @@ -702,7 +702,7 @@ char *virDomainSnapshotDefFormat(const char *domain_uuid,
>>  virBufferAddLit(, "\n");
>>  }
>>  if (def->dom) {
>> -if (virDomainDefFormatInternal(def->dom, flags, ) < 0) {
>> +if (virDomainDefFormatInternal(def->dom, NULL, flags, ) < 0) {
>>  virBufferFreeAndReset();
>>  return NULL;
>>  }
> 
> Yeah, we'll need to pass caps in here too eventually.
I could also add it on this series for v3.

> 
> ACK regardless
Thanks!

> 
> Regards,
> Daniel
> 

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

[libvirt] [PATCH 07/11] qemu: hotplug: Check duplicate disk serial/wwn on hotplug too

[Peter Krempa]

We do the check on VM start, but the user could still hotplug a disk
with a conflicting serial or WWN. Reuse the checker function to fix the
issue.
---
 src/conf/domain_conf.c   | 2 +-
 src/conf/domain_conf.h   | 3 +++
 src/libvirt_private.syms | 1 +
 src/qemu/qemu_hotplug.c  | 3 +++
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2ef6609..8a7a989 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23920,7 +23920,7 @@ virDomainDefNeedsPlacementAdvice(virDomainDefPtr def)
 }


-static int
+int
 virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a,
virDomainDiskDefPtr b)
 {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9fdfdf2..cbf01bf 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3144,6 +3144,9 @@ virDomainParseMemory(const char *xpath,
 bool virDomainDefNeedsPlacementAdvice(virDomainDefPtr def)
 ATTRIBUTE_NONNULL(1);

+int virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a,
+   virDomainDiskDefPtr b)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 int virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def)
 ATTRIBUTE_NONNULL(1);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 69be352..bf25473 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -256,6 +256,7 @@ virDomainDiskByName;
 virDomainDiskCacheTypeFromString;
 virDomainDiskCacheTypeToString;
 virDomainDiskDefAssignAddress;
+virDomainDiskDefCheckDuplicateInfo;
 virDomainDiskDefDstDuplicates;
 virDomainDiskDefForeachPath;
 virDomainDiskDefFree;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2e5cf64..18a5a12 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -799,6 +799,9 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
_("target %s already exists"), disk->dst);
 goto cleanup;
 }
+
+if (virDomainDiskDefCheckDuplicateInfo(vm->def->disks[i], disk) < 
0)
+goto cleanup;
 }

 switch ((virDomainDiskBus) disk->bus) {
-- 
2.6.2

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

[libvirt] [PATCH 00/11] Fix disk dst/wwn/serial checks in hotplug case

[Peter Krempa]

Peter Krempa (11):
  qemu: hotplug: Use typecasted switch
  qemu: hotplug: Remove unnecessary variable
  qemu: hotplug: Break up if/else statement into switch
  qemu: hotplug: Use more common 'cleanup' label in
qemuDomainAttachDeviceDiskLive
  qemu: hotplug: Extract common code to qemuDomainAttachDeviceDiskLive
  conf: Extract code that checks disk serial/wwn conflict
  qemu: hotplug: Check duplicate disk serial/wwn on hotplug too
  qemu: process: Reorder operations on early VM startup
  qemu: process: Extract pre-start checks into a function
  tests: Integrate startup checks to qemuxml2argvtest
  conf: Move and optimize disk target duplicity checking

 src/conf/domain_conf.c| 82 +++--
 src/conf/domain_conf.h|  4 ++-
 src/libvirt_private.syms  |  2 +-
 src/qemu/qemu_command.c   |  3 --
 src/qemu/qemu_hotplug.c   | 85 +--
 src/qemu/qemu_migration.c |  2 +-
 src/qemu/qemu_process.c   | 57 +--
 src/qemu/qemu_process.h   |  9 -
 tests/qemuxml2argvtest.c  | 13 ++--
 9 files changed, 136 insertions(+), 121 deletions(-)

-- 
2.6.2

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

[libvirt] [PATCH 10/11] tests: Integrate startup checks to qemuxml2argvtest

[Peter Krempa]

Some of the tests that are not a part of qemuBuildCommandLine were not
executed in the test suite. We can now reuse qemuProcessStartValidate to
integrate these tests.
---
 tests/qemuxml2argvtest.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a5d4722..af6f9a5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -18,6 +18,7 @@
 # include "qemu/qemu_command.h"
 # include "qemu/qemu_domain.h"
 # include "qemu/qemu_migration.h"
+# include "qemu/qemu_process.h"
 # include "datatypes.h"
 # include "conf/storage_conf.h"
 # include "cpu/cpu_map.h"
@@ -262,6 +263,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
 virCommandPtr cmd = NULL;
 size_t i;
 virBitmapPtr nodeset = NULL;
+bool buildFailed = false;

 if (!(conn = virGetConnect()))
 goto out;
@@ -339,13 +341,20 @@ static int testCompareXMLToArgvFiles(const char *xml,
 goto out;
 }

-if (!(cmd = qemuBuildCommandLine(conn, , vmdef, _chr,
+if (qemuProcessStartValidate(vmdef, extraFlags, !!migrateURI, false) < 0)
+buildFailed = true;
+
+if (!buildFailed &&
+!(cmd = qemuBuildCommandLine(conn, , vmdef, _chr,
  (flags & FLAG_JSON), extraFlags,
  migrateURI, NULL,
  VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
  , false,
  (flags & FLAG_FIPS),
- nodeset, NULL, NULL))) {
+ nodeset, NULL, NULL)))
+buildFailed = true;
+
+if (buildFailed) {
 if (!virtTestOOMActive() &&
 (flags & FLAG_EXPECT_FAILURE)) {
 ret = 0;
-- 
2.6.2

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

Re: [libvirt] [PATCH RFC v2 0/5] conf: add net device prefix capability

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 09:40:32PM +, Joao Martins wrote:
> Hey,
> 
> Back in the pre 1.3.0 release [0] we had a regression when migrating
> domains in libxl (introduced by the reverted commit d2e5538b1).
> 
> The chosen solution to address this problem was to introduce a
> capability to be used by the virDomainNetDefFormat() and
> virDomainNetDefParseXML() routines. RFCv1 implemented this by "caching"
> netprefix in the domain definition[1]; as suggested in the review[1] v2
> implements it by adding the virCapsPtr to the relevant APIs and also
> propagating the API change to all drivers. It is divided as follows:
> 
>   Patch 1: Adds prefix to capabilites
>   Patch 2: Add prefix to NetDefParseXML
>   Patch 3,4: API changes to include virCapsPtr and netprefix (New)
>   Patch 5: set prefix on libxl (Acked)
> 
> Having this series and the reverted commit applied, I can
> successfully migrate a domain without seeing the same interface
> name on both source and destination node with libxl.
> 
> Note: I haven't been able to compile-test all drivers, specifically bhyve,
> vz and hyperv. Testing was made with libxl.

I fixed smalls in the vs & hyperv drivers and pushed this series.

I'll send a followup for the virDomainObjFormat / virDomainSaveStatus
additions.

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

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

[libvirt] [PATCH] conf: add caps to virDomainObjFormat/SaveStatus

[Daniel P. Berrange]

The virDomainObjFormat and virDomainSaveStatus methods
both call into virDomainDefFormat, so should be providing
a non-NULL virCapsPtr instance.

Signed-off-by: Daniel P. Berrange 
---
 src/bhyve/bhyve_process.c   | 15 +++--
 src/conf/domain_conf.c  | 10 +
 src/conf/domain_conf.h  |  4 +++-
 src/libxl/libxl_domain.c|  2 +-
 src/libxl/libxl_driver.c| 14 ++--
 src/libxl/libxl_migration.c |  4 ++--
 src/lxc/lxc_driver.c| 12 +--
 src/lxc/lxc_process.c   |  4 ++--
 src/qemu/qemu_blockjob.c|  2 +-
 src/qemu/qemu_domain.c  |  4 ++--
 src/qemu/qemu_driver.c  | 52 ++---
 src/qemu/qemu_migration.c   |  6 +++---
 src/qemu/qemu_process.c | 36 +++
 13 files changed, 90 insertions(+), 75 deletions(-)

diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 42255d2..9763d71 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -115,11 +115,15 @@ virBhyveProcessStart(virConnectPtr conn,
 bhyveConnPtr privconn = conn->privateData;
 bhyveDomainObjPrivatePtr priv = vm->privateData;
 int ret = -1, rc;
+virCapsPtr caps = NULL;
 
 if (virAsprintf(, "%s/%s.log",
 BHYVE_LOG_DIR, vm->def->name) < 0)
return -1;
 
+caps = bhyveDriverGetCapabilities(privconn);
+if (!caps)
+goto cleanup;
 
 if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT,
   S_IRUSR | S_IWUSR)) < 0) {
@@ -215,12 +219,13 @@ virBhyveProcessStart(virConnectPtr conn,
 
 if (virDomainSaveStatus(driver->xmlopt,
 BHYVE_STATE_DIR,
-vm) < 0)
+vm, caps) < 0)
 goto cleanup;
 
 ret = 0;
 
  cleanup:
+virObjectUnref(caps);
 if (devicemap != NULL) {
 rc = unlink(devmap_file);
 if (rc < 0 && errno != ENOENT)
@@ -362,6 +367,7 @@ virBhyveProcessReconnect(virDomainObjPtr vm,
 char *expected_proctitle = NULL;
 bhyveDomainObjPrivatePtr priv = vm->privateData;
 int ret = -1;
+virCapsPtr caps = NULL;
 
 if (!virDomainObjIsActive(vm))
 return 0;
@@ -369,6 +375,10 @@ virBhyveProcessReconnect(virDomainObjPtr vm,
 if (!vm->pid)
 return 0;
 
+caps = bhyveDriverGetCapabilities(privconn);
+if (!caps)
+return -1;
+
 virObjectLock(vm);
 
 kp = kvm_getprocs(data->kd, KERN_PROC_PID, vm->pid, );
@@ -397,9 +407,10 @@ virBhyveProcessReconnect(virDomainObjPtr vm,
  VIR_DOMAIN_SHUTOFF_UNKNOWN);
 ignore_value(virDomainSaveStatus(data->driver->xmlopt,
  BHYVE_STATE_DIR,
- vm));
+ vm, caps));
 }
 
+virObjectUnref(caps);
 virObjectUnlock(vm);
 VIR_FREE(expected_proctitle);
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 035e5e1..187495c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22516,6 +22516,7 @@ virDomainDefFormat(virDomainDefPtr def, virCapsPtr 
caps, unsigned int flags)
 char *
 virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
virDomainObjPtr obj,
+   virCapsPtr caps,
unsigned int flags)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -22540,7 +22541,7 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
 xmlopt->privateData.format(, obj) < 0)
 goto error;
 
-if (virDomainDefFormatInternal(obj->def, NULL, flags, ) < 0)
+if (virDomainDefFormatInternal(obj->def, caps, flags, ) < 0)
 goto error;
 
 virBufferAdjustIndent(, -2);
@@ -22746,7 +22747,8 @@ virDomainSaveConfig(const char *configDir,
 int
 virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
 const char *statusDir,
-virDomainObjPtr obj)
+virDomainObjPtr obj,
+virCapsPtr caps)
 {
 unsigned int flags = (VIR_DOMAIN_DEF_FORMAT_SECURE |
   VIR_DOMAIN_DEF_FORMAT_STATUS |
@@ -22757,7 +22759,7 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
 int ret = -1;
 char *xml;
 
-if (!(xml = virDomainObjFormat(xmlopt, obj, flags)))
+if (!(xml = virDomainObjFormat(xmlopt, obj, caps, flags)))
 goto cleanup;
 
 if (virDomainSaveXML(statusDir, obj->def, xml))
@@ -23906,7 +23908,7 @@ virDomainObjSetMetadata(virDomainObjPtr vm,
 if (virDomainDefSetMetadata(vm->def, type, metadata, key, uri) < 0)
 return -1;
 
-if (virDomainSaveStatus(xmlopt, stateDir, vm) < 0)
+if (virDomainSaveStatus(xmlopt, stateDir, vm, caps) < 0)
 return -1;
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8843cbd..79060bc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2738,6 +2738,7 

Re: [libvirt] [PATCH RFC v2 3/5] conf: add caps to virDomainDefFormat*

[Daniel P. Berrange]

On Thu, Feb 04, 2016 at 12:57:30PM +, Joao Martins wrote:
> 
> 
> On 02/04/2016 11:14 AM, Daniel P. Berrange wrote:
> >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> >> index 0141009..6ae2e1b 100644
> >> --- a/src/conf/domain_conf.h
> >> +++ b/src/conf/domain_conf.h
> >> @@ -2733,11 +2733,13 @@ void virDomainIOThreadSchedDelId(virDomainDefPtr 
> >> def, unsigned int iothread_id);
> >>  unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
> >>  
> >>  char *virDomainDefFormat(virDomainDefPtr def,
> >> + virCapsPtr caps,
> >>   unsigned int flags);
> >>  char *virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
> >>   virDomainObjPtr obj,
> >>   unsigned int flags);
> > 
> > Surprised we don't have caps added to this one too, since it
> > will call virDomainDefFormat.
> Ah sorry, this one slipped. It would need changing virDomainSaveStatus to
> include caps (and propagating that change to qemu, lxc, libxl and bhyve too) 
> so
> perhaps in a separate patch?

No prob, I just sent a followup to fix that ommission.


> >> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> >> index ea511ec..ffa1bf2 100644
> >> --- a/src/conf/snapshot_conf.c
> >> +++ b/src/conf/snapshot_conf.c
> >> @@ -702,7 +702,7 @@ char *virDomainSnapshotDefFormat(const char 
> >> *domain_uuid,
> >>  virBufferAddLit(, "\n");
> >>  }
> >>  if (def->dom) {
> >> -if (virDomainDefFormatInternal(def->dom, flags, ) < 0) {
> >> +if (virDomainDefFormatInternal(def->dom, NULL, flags, ) < 0) {
> >>  virBufferFreeAndReset();
> >>  return NULL;
> >>  }
> > 
> > Yeah, we'll need to pass caps in here too eventually.
> I could also add it on this series for v3.

If you can coook up a patch for this one too, that'd be great


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 v3 4/4] logical: Display thin lv's found in a libvirt managed volume group

[John Ferlan]

On 02/04/2016 04:53 AM, Ján Tomko wrote:
> On Wed, Feb 03, 2016 at 03:51:30PM -0500, John Ferlan wrote:
>>
>>
>> On 02/03/2016 03:23 AM, Ján Tomko wrote:
>>> On Tue, Feb 02, 2016 at 11:14:01AM -0500, John Ferlan wrote:


 On 02/02/2016 08:30 AM, Pavel Hrdina wrote:
> On Tue, Feb 02, 2016 at 09:11:51AM +0100, Ján Tomko wrote:
>> A thin pool is another layer on top of some of the LVs in the VG.
>> I think it deserves a separate pool type.
>
> I must agree with Jan, and also already wrote the same for v2 patch.  The 
> thin
> pool and thin LV shouldn't be mixed with normal logical pool even they 
> share the
> same VG.
>

 If LVM allows it, then who are we to say it cannot or shouldn't work or
 how it should be configured?
>>>
>>> Even if they are mixed in the same VG, we could show the "thick" LVs
>>> in the type='logical' pool and the thin ones in type='thin'.
>>>
>>
>> We could if our logic to create/build the pool was overhauled or perhaps
>> support was added to have two volume groups use the same source device.
>>
>> Since it's possible to place a thin_pool in the same vg as a snapshot
>> and a "thick" lv, should we be in the business of splitting hairs and
>> trying to define how things should be viewed?
>>
> 
> We already are. E.g. in the 'fs' pool, we ignore fifos and subdirectories.
> 

True we don't follow any subdir for an 'fs' pool (or 'dir' pool);
however, a 'thin-pool' and 'thin' lv are both in a VG. The hierarchy is
managed by LVM. If you looked at the on disk structure you'd only find
the 'thin' subdir, but there is no 'thin-pool' subdir.


>> I don't see the benefit behind requiring the user to decide whether to
>> have a libvirt pool view either thin lv's or non-thin lv's of their vg
>> or requiring their vg to essentially be one thin-pool in order to
>> support thin lv's, when we could support whatever configuration they've
>> already developed.
>>
> 
> The thin pool would view the thin volumes in a specific thin pool, not
> all thin pools in that VG.
> 

Sure I understand the goal - a 1-to-1 relationship between a thin-pool
in a volume group and the libvirt pool. Then a 1-to-n relationship
between the pool and it's thin lv's. If a VG had two thin-pools, then
each would have to be it's own libvirt pool.

Configuring the libvirt specific pool is an extra step or two. The
libvirt pool would list the thin-pool size as capacity and the
allocation could be based on the data_percent. Perhaps the one benefit I
can see from this model.

I do see a lot of extra code, documentation, and configuration steps.

>> If more work is done in LV pools - are we going to create new pool types
>> for other lvm segtypes ("mirror", "stripe", "raid", etc.)? In order to
>> display/fetch interesting or specific things about them?
>>
> 
> AFAIK none of these create another pool on top of the VG.
> 

True - not in the same manner as a thin-pool and thin lv have an overt
relationship.  However, thin lv's and thin-pool's are listed within a VG
not on top of a VG. The thin-pool is just the mechanism used to define
the physical storage extents from which the virtual 'thin' extents can
be drawn from. If the thin-pool runs out of extents, the admin must come
along and extend it. That is - libvirt is not involved. No different
than if a VG (without a thin-pool) ran out of space. The admin would
need to extend it via some LVM command. Again libvirt is hands off.

I see creating a separate pool as a needless hierarchical step for the
primary benefit of being able to display the capacity of the thin-pool.
I think it's easier to describe that a logical pool that contains thin
lv's can appear to be over-subscribed. Management of the thin-pool's
from which those thin lv's draw from is left to LVM similar to whatever
configuration steps are required to create a device path for an 'fs'
pool to use.

John

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

Re: [libvirt] [PATCH v3 3/8] virConnectCloseCallbackData: factor out callback disarming

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:16PM +0300, Nikolay Shirokovskiy wrote:
> ---
>  src/datatypes.c | 31 ---
>  1 file changed, 16 insertions(+), 15 deletions(-)

ACK


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

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

Re: [libvirt] [PATCH 1/3] Revert "systemd: Escape only needed characters for machined"

[Martin Kletzander]

On Wed, Feb 03, 2016 at 09:31:50PM +0100, Andrea Bolognani wrote:

On Wed, 2016-02-03 at 14:07 +0100, Pavel Hrdina wrote:

On Tue, Feb 02, 2016 at 09:22:47PM +0100, Martin Kletzander wrote:
> This reverts commit 0e0149ce91d84f40b98acf4c4bb0da6e29b9c15c.
> 
> That commit was added to comply with systemd rules that were changed in
> the meantime, so this patch is pointless.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/util/virsystemd.c  | 23 +++
>  tests/virsystemdtest.c |  4 ++--
>  2 files changed, 9 insertions(+), 18 deletions(-)
 
ACK


I have a guest named debian-q35, and after this commit I can no
longer start it:

  error: Failed to start domain debian-q35
  error: Invalid machine name

Looks like the name gets escaped as

 qemu-debian\x2dq35

and systemd doesn't like it. After reverting your revert I can
start the guest again. I'm running

  systemd-222-13.fc23.x86_64

which I assume is pretty recent.



Sorry for that, I completely forgot that in my latest series the first
two patches are dependent on the last one.  I'm trying never to do that,
but this time I wanted to split it so that it's nice to review, polus
the revert was there...

Anyway, really sorry for that, I'm just now testing the (hopefully) last
version of the third patch, will post it really soon.


Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/3] libxl: add p2p migration

[Jim Fehlig]

On 02/03/2016 12:36 PM, Joao Martins wrote:
>
> On 02/02/2016 11:41 PM, Jim Fehlig wrote:
>> Also, if the connection dies, how are we informed about that?
>> Would that be done via the connect close callback that was removed in this
>> version? It is not clear to me how errors on the dconn connection are 
>> handled.
>>
> In the close callback, we're just notified that the connection got closed, 
> but I
> am not sure if there is much we can do about it in the handler.
> We could set an error on the callback, and then check dconn callers on
> libxlDoMigrateP2P() before any RPC calls?

Ah, nevermind. If the connection got closed, the dconn calls will fail and
errors will be handled at the call sites.

Regards,
Jim

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

[libvirt] [PATCH 09/11] qemu: process: Extract pre-start checks into a function

[Peter Krempa]

When starting a qemu process there are certain checks done to ensure
that the configuration makes sense. Extract them into a separate
function so that they can be reused in the test code.
---
 src/qemu/qemu_migration.c |  2 +-
 src/qemu/qemu_process.c   | 41 -
 src/qemu/qemu_process.h   |  9 -
 3 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 51e7125..c13e1b5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3552,7 +3552,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 goto stopjob;
 }

-if (qemuProcessInit(driver, vm, true) < 0)
+if (qemuProcessInit(driver, vm, true, false) < 0)
 goto stopjob;
 stopProcess = true;

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0f617da..ea1e103 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4401,6 +4401,32 @@ qemuProcessMakeDir(virQEMUDriverPtr driver,


 /**
+ * qemuProcessStartValidate:
+ * @vm: domain object
+ * @qemuCaps: emulator capabilities
+ * @migration: restoration of eixting state
+ *
+ * This function agregates checks independent from host state done prior to
+ * start of a VM.
+ */
+int
+qemuProcessStartValidate(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ bool migration,
+ bool snapshot)
+{
+if (qemuValidateCpuCount(def, qemuCaps) < 0)
+return -1;
+
+if (!migration && !snapshot &&
+virDomainDefCheckDuplicateDiskInfo(def) < 0)
+return -1;
+
+return 0;
+}
+
+
+/**
  * qemuProcessInit:
  *
  * Prepares the domain up to the point when priv->qemuCaps is initialized. The
@@ -4411,7 +4437,8 @@ qemuProcessMakeDir(virQEMUDriverPtr driver,
 int
 qemuProcessInit(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
-bool migration)
+bool migration,
+bool snap)
 {
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 virCapsPtr caps = NULL;
@@ -4440,6 +4467,9 @@ qemuProcessInit(virQEMUDriverPtr driver,
   vm->def->os.machine)))
 goto cleanup;

+if (qemuProcessStartValidate(vm->def, priv->qemuCaps, migration, snap) < 0)
+goto cleanup;
+
 /* Some things, paths, ... are generated here and we want them to persist.
  * Fill them in prior to setting the domain def as transient. */
 VIR_DEBUG("Generating paths");
@@ -4640,9 +4670,6 @@ qemuProcessLaunch(virConnectPtr conn,
 }
 }

-if (qemuValidateCpuCount(vm->def, priv->qemuCaps) < 0)
-goto cleanup;
-
 if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0)
 goto cleanup;

@@ -4666,10 +4693,6 @@ qemuProcessLaunch(virConnectPtr conn,
 goto cleanup;
 }

-if (!incoming && !snapshot &&
-virDomainDefCheckDuplicateDiskInfo(vm->def) < 0)
-goto cleanup;
-
 /* "volume" type disk's source must be translated before
  * cgroup and security setting.
  */
@@ -5112,7 +5135,7 @@ qemuProcessStart(virConnectPtr conn,
   VIR_QEMU_PROCESS_START_PAUSED |
   VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup);

-if (qemuProcessInit(driver, vm, !!migrateFrom) < 0)
+if (qemuProcessInit(driver, vm, !!migrateFrom, !!snapshot) < 0)
 goto cleanup;

 if (migrateFrom) {
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index cb5cee1..907a58d 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -81,9 +81,16 @@ int qemuProcessStart(virConnectPtr conn,
  virNetDevVPortProfileOp vmop,
  unsigned int flags);

+
+int qemuProcessStartValidate(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ bool migration,
+ bool snap);
+
 int qemuProcessInit(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
-bool migration);
+bool migration,
+bool snap);

 int qemuProcessLaunch(virConnectPtr conn,
   virQEMUDriverPtr driver,
-- 
2.6.2

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

[libvirt] [PATCH 01/11] qemu: hotplug: Use typecasted switch

[Peter Krempa]

Remove the default case since all cases are covered.
---
 src/qemu/qemu_hotplug.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f8db960..b456fed 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -795,7 +795,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
 goto end;

-switch (disk->device)  {
+switch ((virDomainDiskDevice) disk->device)  {
 case VIR_DOMAIN_DISK_DEVICE_CDROM:
 case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
 if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
@@ -837,10 +837,8 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
virDomainDiskBusTypeToString(disk->bus));
 }
 break;
-default:
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-   _("disk device type '%s' cannot be hotplugged"),
-   virDomainDiskDeviceTypeToString(disk->device));
+
+case VIR_DOMAIN_DISK_DEVICE_LAST:
 break;
 }

-- 
2.6.2

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

Re: [libvirt] [PATCH v3 1/8] factor out virConnectCloseCallbackDataPtr methods

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:14PM +0300, Nikolay Shirokovskiy wrote:
> Signed-off-by: Nikolay Shirokovskiy 
> ---
>  po/POTFILES.in |  1 +
>  src/datatypes.c| 75 
> ++
>  src/datatypes.h| 10 +++
>  src/libvirt-host.c | 35 ++
>  src/remote/remote_driver.c | 17 ++-
>  5 files changed, 91 insertions(+), 47 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH v3 6/8] close callback: move it to driver

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:19PM +0300, Nikolay Shirokovskiy wrote:
> Signed-off-by: Nikolay Shirokovskiy 
> ---
>  src/datatypes.c| 31 ++
>  src/datatypes.h|  1 +
>  src/driver-hypervisor.h| 12 
>  src/libvirt-host.c |  7 ---
>  src/remote/remote_driver.c | 47 
> ++
>  5 files changed, 70 insertions(+), 28 deletions(-)

ACK


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

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

[libvirt] [PATCH] Fix build after recent patches

[Peter Krempa]

Few build breaking mistakes in less-popular parts of our code.
---

Pushed under the build-breaker rule.

 src/security/security_apparmor.c | 2 +-
 src/xenapi/xenapi_driver.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 2cf333d..af2b639 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -181,7 +181,7 @@ load_profile(virSecurityManagerPtr mgr,
 const char *probe = virSecurityManagerGetAllowDiskFormatProbing(mgr)
 ? "1" : "0";

-xml = virDomainDefFormat(def, VIR_DOMAIN_DEF_FORMAT_SECURE);
+xml = virDomainDefFormat(def, NULL, VIR_DOMAIN_DEF_FORMAT_SECURE);
 if (!xml)
 goto cleanup;

diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index c4f33b9..f75f138 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1402,7 +1402,7 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int 
flags)
 xen_vm vm = NULL;
 xen_vm_set *vms;
 xen_string_string_map *result = NULL;
-struct _xenapiPrivate *priv = conn->privateData;
+struct _xenapiPrivate *priv = dom->conn->privateData;
 xen_session *session = priv->session;
 virDomainDefPtr defPtr = NULL;
 char *boot_policy = NULL;
-- 
2.6.2

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

Re: [libvirt] [PATCH RFC 2/8] Introduce virStream{Recv,Send}Offset

[Lee Schermerhorn]

On Mon, 2016-02-01 at 14:49 +, Daniel P. Berrange wrote:
> On Fri, Jan 29, 2016 at 02:26:53PM +0100, Michal Privoznik wrote:
> > When dealing with sparse files we need to be able to jump over
> > holes as there's no added value in reading/writing them. For
> > that, we need new set of send and receive APIs that will have
> > @offset argument. Sending data to a stream would be easy - just
> > say from which offset we are sending data. Reading is a bit
> > tricky - we need read function which can detect holes and thus
> > when requested to read from one it will set @offset to new value
> > that contains data.
> > 
> > Signed-off-by: Michal Privoznik 
> > ---
> >  include/libvirt/libvirt-stream.h |   8 +++
> >  src/driver-stream.h  |  13 +
> >  src/libvirt-stream.c | 113
> > +++
> >  src/libvirt_public.syms  |   6 +++
> >  4 files changed, 140 insertions(+)
> > 
> > diff --git a/include/libvirt/libvirt-stream.h
> > b/include/libvirt/libvirt-stream.h
> > index 831640d..5a2bde3 100644
> > --- a/include/libvirt/libvirt-stream.h
> > +++ b/include/libvirt/libvirt-stream.h
> > @@ -40,11 +40,19 @@ int virStreamRef(virStreamPtr st);
> >  int virStreamSend(virStreamPtr st,
> >    const char *data,
> >    size_t nbytes);
> > +int virStreamSendOffset(virStreamPtr stream,
> > +unsigned long long offset,
> > +const char *data,
> > +size_t nbytes);
> >  
> >  int virStreamRecv(virStreamPtr st,
> >    char *data,
> >    size_t nbytes);
> >  
> > +int virStreamRecvOffset(virStreamPtr stream,
> > +unsigned long long *offset,
> > +char *data,
> > +size_t nbytes);
> >  
> >  /**
> >   * virStreamSourceFunc:
> > diff --git a/src/driver-stream.h b/src/driver-stream.h
> > index 85b4e3b..5419b85 100644
> > --- a/src/driver-stream.h
> > +++ b/src/driver-stream.h
> > @@ -31,9 +31,20 @@ typedef int
> >  size_t nbytes);
> >  
> >  typedef int
> > +(*virDrvStreamSendOffset)(virStreamPtr st,
> > +  unsigned long long offset,
> > +  const char *data,
> > +  size_t nbytes);
> > +
> > +typedef int
> >  (*virDrvStreamRecv)(virStreamPtr st,
> >  char *data,
> >  size_t nbytes);
> > +typedef int
> > +(*virDrvStreamRecvOffset)(virStreamPtr st,
> > +  unsigned long long *offset,
> > +  char *data,
> > +  size_t nbytes);
> >  
> >  typedef int
> >  (*virDrvStreamEventAddCallback)(virStreamPtr stream,
> > @@ -60,7 +71,9 @@ typedef virStreamDriver *virStreamDriverPtr;
> >  
> >  struct _virStreamDriver {
> >  virDrvStreamSend streamSend;
> > +virDrvStreamSendOffset streamSendOffset;
> >  virDrvStreamRecv streamRecv;
> > +virDrvStreamRecvOffset streamRecvOffset;
> >  virDrvStreamEventAddCallback streamEventAddCallback;
> >  virDrvStreamEventUpdateCallback streamEventUpdateCallback;
> >  virDrvStreamEventRemoveCallback streamEventRemoveCallback;
> > diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
> > index c16f586..1df188c 100644
> > --- a/src/libvirt-stream.c
> > +++ b/src/libvirt-stream.c
> > @@ -192,6 +192,58 @@ virStreamSend(virStreamPtr stream,
> >  
> >  
> >  /**
> > + * virStreamSendOffset:
> > + * @stream: pointer to the stream object
> > + * @offset: 
> > + * @data: buffer to write to stream
> > + * @nbytes: size of @data buffer
> > + *
> > + * Sends some data down the pipe.
> > + *
> > + * Returns the number of bytes written, which may be less
> > + * than requested.
> > + *
> > + * Returns -1 upon error, at which time the stream will
> > + * be marked as aborted, and the caller should now release
> > + * the stream with virStreamFree.
> > + *
> > + * Returns -2 if the outgoing transmit buffers are full &
> > + * the stream is marked as non-blocking.
> > + */
> > +int
> > +virStreamSendOffset(virStreamPtr stream,
> > +unsigned long long offset,
> > +const char *data,
> > +size_t nbytes)
> > +{
> > +VIR_DEBUG("stream=%p, offset=%llu, data=%p, nbytes=%zu",
> > +  stream, offset, data, nbytes);
> > +
> > +virResetLastError();
> > +
> > +virCheckStreamReturn(stream, -1);
> > +virCheckNonNullArgGoto(data, error);
> > +
> > +if (stream->driver &&
> > +stream->driver->streamSendOffset) {
> > +int ret;
> > +ret = (stream->driver->streamSendOffset)(stream, offset,
> > data, nbytes);
> > +if (ret == -2)
> > +return -2;
> > +if (ret < 0)
> > +goto error;
> > +return ret;
> > +}
> > +
> > +virReportUnsupportedError();
> > +
> > + error:
> > +

Re: [libvirt] [PATCH v3 2/8] virConnectCloseCallbackData: fix connection object refcount

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:15PM +0300, Nikolay Shirokovskiy wrote:
> We have reference to connection object in virConnectCloseCallbackData
> object thus we have to refcount it. Obviously we have problems
> in dispose and call functions. Let's fix it.
> 
> Signed-off-by: Nikolay Shirokovskiy 
> ---
>  src/datatypes.c | 4 
>  1 file changed, 4 insertions(+)

ACK


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

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

Re: [libvirt] [PATCH v3 1/7] qemu migration: factor out setting migration option

[Jiri Denemark]

On Thu, Jan 28, 2016 at 10:04:27 +0300, Nikolay Shirokovskiy wrote:
> Signed-off-by: Nikolay Shirokovskiy 
> ---
>  src/qemu/qemu_migration.c | 138 
> ++
>  1 file changed, 28 insertions(+), 110 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 51e7125..a6412ce 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
...
> @@ -2489,7 +2402,7 @@ qemuMigrationSetPinAll(virQEMUDriverPtr driver,
>  
>  ret = qemuMonitorSetMigrationCapability(
>  priv->mon,
> -QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL,
> +capability,
>  state);

All the arguments will fit on a single line now.

ACK and pushed since this patch is nice to have independently of the
rest of this series.

Jirka

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

Re: [libvirt] [PATCH v3 4/8] close callback API: remove unnecessary locks

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:17PM +0300, Nikolay Shirokovskiy wrote:
> closeCallback pointer is immutable (set on connection object creation)
> and self-locking.
> ---
>  src/libvirt-host.c | 10 --
>  1 file changed, 10 deletions(-)

ACK


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

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

Re: [libvirt] [PATCH v3 5/8] virConnectCloseCallbackDataDispose: remove unnecessary locks

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:18PM +0300, Nikolay Shirokovskiy wrote:
> We don't need locks in dispose functions as they can only
> be run in one thread for given object.
> ---
>  src/datatypes.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)

ACK


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

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

Re: [libvirt] [PATCH v3 8/8] vz: implement connection close notification

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:21PM +0300, Nikolay Shirokovskiy wrote:
> Signed-off-by: Nikolay Shirokovskiy 
> ---
>  src/vz/vz_driver.c | 39 +++
>  src/vz/vz_sdk.c|  4 
>  src/vz/vz_utils.h  |  3 +++
>  3 files changed, 46 insertions(+)

ACK


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

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

Re: [libvirt] [PATCH v3 7/8] daemon: add connection close rpc

[Daniel P. Berrange]

On Mon, Feb 01, 2016 at 03:40:20PM +0300, Nikolay Shirokovskiy wrote:
> remoteConnectUnregisterCloseCallback is not quite good.
> if it is given a callback function different from that
> was registered before then local part will fail silently. On
> the other hand we can not gracefully handle this fail
> as the remote part is already unregistered.

We could sanity check the callback before unregistering the
remote part. Or you could do the local unregister first since
if the remote part then fails, it is harmless - we'll see the
close event frm the server still, but we won't dispatch it.

> There are a lot of options to fix it. I think of totally
> removing the callback argument from unregistering. What's
> the use of it?

We can't remove it since that would change ABI.

> ---
>  daemon/libvirtd.h|  1 +
>  daemon/remote.c  | 84 
> 
>  src/remote/remote_driver.c   | 52 ---
>  src/remote/remote_protocol.x | 24 -
>  src/remote_protocol-structs  |  6 
>  5 files changed, 161 insertions(+), 6 deletions(-)
> 
> diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
> index efd4823..7271b0f 100644
> --- a/daemon/libvirtd.h
> +++ b/daemon/libvirtd.h
> @@ -60,6 +60,7 @@ struct daemonClientPrivate {
>  size_t nnetworkEventCallbacks;
>  daemonClientEventCallbackPtr *qemuEventCallbacks;
>  size_t nqemuEventCallbacks;
> +bool closeRegistered;
>  
>  # if WITH_SASL
>  virNetSASLSessionPtr sasl;
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 370f442..bea1996 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -1221,6 +1221,20 @@ remoteRelayDomainQemuMonitorEvent(virConnectPtr conn,
>  VIR_FREE(details_p);
>  }
>  
> +static
> +void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, 
> int reason, void *opaque)
> +{
> +virNetServerClientPtr client = opaque;
> +
> +VIR_DEBUG("Relaying connection closed event, reason %d", reason);
> +
> +remote_connect_event_connection_closed_msg msg = { reason };
> +remoteDispatchObjectEventSend(client, remoteProgram,
> +  
> REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED,
> +  
> (xdrproc_t)xdr_remote_connect_event_connection_closed_msg,
> +  );
> +}
> +
>  /*
>   * You must hold lock for at least the client
>   * We don't free stuff here, merely disconnect the client's
> @@ -1283,6 +1297,12 @@ void remoteClientFreeFunc(void *data)
>  }
>  VIR_FREE(priv->qemuEventCallbacks);
>  
> +if (priv->closeRegistered) {
> +if (virConnectUnregisterCloseCallback(priv->conn,
> +  
> remoteRelayConnectionClosedEvent) < 0)
> +VIR_WARN("unexpected close callback event deregister 
> failure");
> +}
> +
>  virConnectClose(priv->conn);
>  
>  virIdentitySetCurrent(NULL);
> @@ -3515,6 +3535,70 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr 
> server ATTRIBUTE_UNUSED,
>  return rv;
>  }
>  
> +static int
> +remoteDispatchConnectCloseCallbackRegister(virNetServerPtr server 
> ATTRIBUTE_UNUSED,
> +   virNetServerClientPtr client,
> +   virNetMessagePtr msg 
> ATTRIBUTE_UNUSED,
> +   virNetMessageErrorPtr rerr)
> +{
> +int rv = -1;
> +struct daemonClientPrivate *priv =
> +virNetServerClientGetPrivateData(client);
> +
> +virMutexLock(>lock);
> +
> +if (!priv->conn) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not 
> open"));
> +goto cleanup;
> +}
> +
> +// on behalf of close callback
> +virObjectRef(client);
> +if (virConnectRegisterCloseCallback(priv->conn,
> +remoteRelayConnectionClosedEvent,
> +client, virObjectFreeCallback) < 0)
> +goto cleanup;
> +
> +priv->closeRegistered = true;
> +rv = 0;
> +
> + cleanup:
> +virMutexUnlock(>lock);
> +if (rv < 0)
> +virNetMessageSaveError(rerr);
> +return rv;
> +}
> +
> +static int
> +remoteDispatchConnectCloseCallbackUnregister(virNetServerPtr server 
> ATTRIBUTE_UNUSED,
> + virNetServerClientPtr client,
> + virNetMessagePtr msg 
> ATTRIBUTE_UNUSED,
> + virNetMessageErrorPtr rerr)
> +{
> +int rv = -1;
> +struct daemonClientPrivate *priv =
> +virNetServerClientGetPrivateData(client);
> +
> +virMutexLock(>lock);
> +
> +if (!priv->conn) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not 
> open"));
> +goto cleanup;
> +}
> +
> +if (virConnectUnregisterCloseCallback(priv->conn,

Re: [libvirt] [PATCH] libxl: support feature

[Jim Fehlig]

On 02/04/2016 05:54 AM, Joao Martins wrote:
>
> On 02/04/2016 01:41 AM, Jim Fehlig wrote:
>> On 02/03/2016 02:20 PM, Joao Martins wrote:
>>> On 02/03/2016 04:12 AM, Jim Fehlig wrote:
 Even though the libxl driver advertises  in capabilities,
 it is ignored when set in domXML. Enable hap in the
 libxl_domain_create_info struct when  is specified in domXML.

 The xm/xl <-> domXML parser/formatter already supports hap but
 lacked a test with hap enabled.  Add a hap test.
>> [...]
>>> FWIW looks good and also:
>>>
>>> Tested-by: Joao Martins 
>> Thanks for reviewing/testing the patch! But after playing with the patch a 
>> bit
>> more, I'm considering dropping it :-/.
>>
>> Before the patch,  was ignored by the libxl driver, which left the 'hap'
>> field of libxl_domain_create_info struct unset (= LIBXL__DEFBOOL_DEFAULT). 
>> For
>> HVM guests, libxl will enable hap when libxl_domain_create_info.hap ==
>> LIBXL__DEFBOOL_DEFAULT. And using hap is good, since using shadow page table 
>> is
>> less efficient. The downside is there is no way to turn hap off.
>>
>> After the patch it is possible to turn hap on and off via presence or 
>> absence of
>> . However, there is a *lot* of existing config without  that 
>> currently
>> reaps the benefits of hap nonetheless. After patch, that config would require
>> adding , else shadow page table will be used. (Note that in xm and xl
>> config, hap is enabled in the absence of 'hap='.)
>>
>> I'm starting to question whether this feature should even be exposed. I think
>> the only use case is debugging. E.g. turn hap off if suspected hardware bug.
>> Otherwise you would certainly want the hypervisor to use hap if it is able 
>> to do
>> so. What do you think?
>>
> Hm, good point. There is another field for VIR_TRISTATE which represents when
> it's absent (VIR_TRISTATE_SWITCH_ABSENT). So perhaps we would leave the 
> default
> (= LIBXL_DEFBOOL_DEFAULT) if it's absent, and only set hap if the user
> explicitly switches on/off?

But IIUC, absent == off. I.e., there is no way to turn hap off except the
absence of .

One option might be to extend  with an 'enabled=yes|no' attribute. Then the
absence of  means hypervisor default.  without the attribute would be
the same as  for backwards compatibility. And of course  disables hap.

Note that capabilities currently advertises hap as

  

If folks agree to this option, the default should be changed to 'on'.

>  I am not sure if the usecase is just debug, but
> given there are certain performance differences compared to shadow paging
> (depending on the HVM guest workload) it might be nice to allow the
> administrator to choose it.

Agreed. I think the above option accomplishes that.

Regards,
Jim

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

Re: [libvirt] [PATCH v3 3/7] qemu monitor: add multithread compress parameters accessors

[Jiri Denemark]

On Thu, Jan 28, 2016 at 10:04:29 +0300, Nikolay Shirokovskiy wrote:
> From: ShaoHe Feng 
> 
> Current compression does not use all range of parameter values
> so let's use some of them as 'unspecified' values. These
> values will be used to mark parameters that were not specified
> on migrate command line. Thus we check that qemu does not
> use these values when we read parameters.
> 
> Signed-off-by: ShaoHe Feng 
> Signed-off-by: Nikolay Shirokovskiy 
> ---
>  src/qemu/qemu_monitor.c  |  29 +
>  src/qemu/qemu_monitor.h  |  16 +++
>  src/qemu/qemu_monitor_json.c |  87 +
>  src/qemu/qemu_monitor_json.h |   5 +++
>  src/qemu/qemu_monitor_text.c | 100 
> +++
>  src/qemu/qemu_monitor_text.h |   5 +++

Please, remove everything related to HMP monitor, multithreaded
compression is not supported by any QEMU for which we would use text
monitor.

>  6 files changed, 242 insertions(+)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index cf1cdfb..e9b1ce4 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2116,6 +2116,35 @@ qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon,
>  
>  
>  int
> +qemuMonitorGetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +
> qemuMonitorMigrationMTParametersPtr params)

I think a more general qemuMonitor[GS]etMigrationParameters [gs]etting
all parameters at once would be a bit better. After all, it all boils
down to query-migrate-parameters and migrate-set-parameters and having
separate functions for each group of parameters would mean we'd have to
call the QMP commands several times.

> +{
> +QEMU_CHECK_MONITOR(mon);
> +
> +if (mon->json)
> +return qemuMonitorJSONGetMigrationCompressParametersMT(mon, params);
> +else
> +return qemuMonitorTextGetMigrationCompressParametersMT(mon, params);

Since support for text monitor is not desirable, this function will be
as simple as

QEMU_CHECK_MONITOR_JSON(mon);

return qemuMonitorJSON...;

> +}
> +
> +int
> +qemuMonitorSetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +
> qemuMonitorMigrationMTParametersPtr params)
> +{
> +VIR_DEBUG("level=%d threads=%d dthreads=%d", params->level,
> + params->threads,
> + params->dthreads);

Either

VIR_DEBUG("level=%d threads=%d dthreads=%d",
  params->level,
  params->threads,
  params->dthreads);

or

VIR_DEBUG("level=%d threads=%d dthreads=%d",
  params->level, params->threads, params->dthreads);

> +
> +QEMU_CHECK_MONITOR(mon);
> +
> +if (mon->json)
> +return qemuMonitorJSONSetMigrationCompressParametersMT(mon, params);
> +else
> +return qemuMonitorTextSetMigrationCompressParametersMT(mon, params);
> +}
> +
> +
> +int
>  qemuMonitorGetMigrationStats(qemuMonitorPtr mon,
>   qemuMonitorMigrationStatsPtr stats)
>  {
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index c2a0ed6..5a5e0e2 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -463,6 +463,22 @@ int qemuMonitorGetMigrationCacheSize(qemuMonitorPtr mon,
>  int qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon,
>   unsigned long long cacheSize);
>  
> +typedef struct _qemuMonitorMigrationMTParameters 
> qemuMonitorMigrationMTParameters;
> +typedef qemuMonitorMigrationMTParameters 
> *qemuMonitorMigrationMTParametersPtr;
> +struct _qemuMonitorMigrationMTParameters {
> +/* -1 is value of unspecified */
> +int level;
> +/* 0 is value of unspecified */
> +unsigned int threads;
> +/* 0 is value of unspecified */
> +unsigned int dthreads;
> +};
> +
> +int qemuMonitorGetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +
> qemuMonitorMigrationMTParametersPtr params);
> +int qemuMonitorSetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +
> qemuMonitorMigrationMTParametersPtr params);
> +
>  typedef enum {
>  QEMU_MONITOR_MIGRATION_STATUS_INACTIVE,
>  QEMU_MONITOR_MIGRATION_STATUS_SETUP,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 24a8865..62aba88 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2437,6 +2437,93 @@ qemuMonitorJSONSetMigrationCacheSize(qemuMonitorPtr 
> mon,
>  }
>  
>  
> +int qemuMonitorJSONGetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +
> qemuMonitorMigrationMTParametersPtr params)
> +{
> +int ret = -1;
> +virJSONValuePtr result;

Re: [libvirt] [PATCH] conf: add caps to virDomainObjFormat/SaveStatus

[Joao Martins]

On 02/04/2016 02:52 PM, Daniel P. Berrange wrote:
> The virDomainObjFormat and virDomainSaveStatus methods
> both call into virDomainDefFormat, so should be providing
> a non-NULL virCapsPtr instance.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/bhyve/bhyve_process.c   | 15 +++--
>  src/conf/domain_conf.c  | 10 +
>  src/conf/domain_conf.h  |  4 +++-
>  src/libxl/libxl_domain.c|  2 +-
>  src/libxl/libxl_driver.c| 14 ++--
>  src/libxl/libxl_migration.c |  4 ++--
>  src/lxc/lxc_driver.c| 12 +--
>  src/lxc/lxc_process.c   |  4 ++--
>  src/qemu/qemu_blockjob.c|  2 +-
>  src/qemu/qemu_domain.c  |  4 ++--
>  src/qemu/qemu_driver.c  | 52 
> ++---
>  src/qemu/qemu_migration.c   |  6 +++---
>  src/qemu/qemu_process.c | 36 +++
>  13 files changed, 90 insertions(+), 75 deletions(-)
> 
> diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
> index 42255d2..9763d71 100644
> --- a/src/bhyve/bhyve_process.c
> +++ b/src/bhyve/bhyve_process.c
> @@ -115,11 +115,15 @@ virBhyveProcessStart(virConnectPtr conn,
>  bhyveConnPtr privconn = conn->privateData;
>  bhyveDomainObjPrivatePtr priv = vm->privateData;
>  int ret = -1, rc;
> +virCapsPtr caps = NULL;
>  
>  if (virAsprintf(, "%s/%s.log",
>  BHYVE_LOG_DIR, vm->def->name) < 0)
> return -1;
>  
> +caps = bhyveDriverGetCapabilities(privconn);
> +if (!caps)
> +goto cleanup;
>  
>  if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT,
>S_IRUSR | S_IWUSR)) < 0) {
> @@ -215,12 +219,13 @@ virBhyveProcessStart(virConnectPtr conn,
>  
>  if (virDomainSaveStatus(driver->xmlopt,
>  BHYVE_STATE_DIR,
> -vm) < 0)
> +vm, caps) < 0)
>  goto cleanup;
>  
>  ret = 0;
>  
>   cleanup:
> +virObjectUnref(caps);
>  if (devicemap != NULL) {
>  rc = unlink(devmap_file);
>  if (rc < 0 && errno != ENOENT)
> @@ -362,6 +367,7 @@ virBhyveProcessReconnect(virDomainObjPtr vm,
>  char *expected_proctitle = NULL;
>  bhyveDomainObjPrivatePtr priv = vm->privateData;
>  int ret = -1;
> +virCapsPtr caps = NULL;
>  
>  if (!virDomainObjIsActive(vm))
>  return 0;
> @@ -369,6 +375,10 @@ virBhyveProcessReconnect(virDomainObjPtr vm,
>  if (!vm->pid)
>  return 0;
>  
> +caps = bhyveDriverGetCapabilities(privconn);
> +if (!caps)
> +return -1;
> +
>  virObjectLock(vm);
>  
>  kp = kvm_getprocs(data->kd, KERN_PROC_PID, vm->pid, );
> @@ -397,9 +407,10 @@ virBhyveProcessReconnect(virDomainObjPtr vm,
>   VIR_DOMAIN_SHUTOFF_UNKNOWN);
>  ignore_value(virDomainSaveStatus(data->driver->xmlopt,
>   BHYVE_STATE_DIR,
> - vm));
> + vm, caps));
>  }
>  
> +virObjectUnref(caps);
>  virObjectUnlock(vm);
>  VIR_FREE(expected_proctitle);
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 035e5e1..187495c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -22516,6 +22516,7 @@ virDomainDefFormat(virDomainDefPtr def, virCapsPtr 
> caps, unsigned int flags)
>  char *
>  virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
> virDomainObjPtr obj,
> +   virCapsPtr caps,
> unsigned int flags)
>  {
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -22540,7 +22541,7 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
>  xmlopt->privateData.format(, obj) < 0)
>  goto error;
>  
> -if (virDomainDefFormatInternal(obj->def, NULL, flags, ) < 0)
> +if (virDomainDefFormatInternal(obj->def, caps, flags, ) < 0)
>  goto error;
>  
>  virBufferAdjustIndent(, -2);
> @@ -22746,7 +22747,8 @@ virDomainSaveConfig(const char *configDir,
>  int
>  virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
>  const char *statusDir,
> -virDomainObjPtr obj)
> +virDomainObjPtr obj,
> +virCapsPtr caps)
>  {
>  unsigned int flags = (VIR_DOMAIN_DEF_FORMAT_SECURE |
>VIR_DOMAIN_DEF_FORMAT_STATUS |
> @@ -22757,7 +22759,7 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
>  int ret = -1;
>  char *xml;
>  
> -if (!(xml = virDomainObjFormat(xmlopt, obj, flags)))
> +if (!(xml = virDomainObjFormat(xmlopt, obj, caps, flags)))
>  goto cleanup;
>  
>  if (virDomainSaveXML(statusDir, obj->def, xml))
> @@ -23906,7 +23908,7 @@ virDomainObjSetMetadata(virDomainObjPtr vm,
>  if (virDomainDefSetMetadata(vm->def, type, metadata, key, uri) < 0)
>  return -1;
>  
> -

Re: [libvirt] [PATCH] logical: Clarify pieces of lvs regex

[Andrea Bolognani]

On Wed, 2016-02-03 at 16:49 -0500, John Ferlan wrote:
> Rather than have a unwieldy regex string - split it up into its components
> each having it's own #define and then combine in a different #define
> 
> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend_logical.c | 32 ++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage/storage_backend_logical.c 
> b/src/storage/storage_backend_logical.c
> index eb22fd0..ba26223 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -312,6 +312,34 @@ virStorageBackendLogicalMakeVol(char **const groups,
>  return ret;
>  }
>  
> +#define VIR_STORAGE_VOL_LOGICAL_PREFIX_REGEX "^\\s*"
> +#define VIR_STORAGE_VOL_LOGICAL_LV_NAME_REGEX "(\\S+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_ORIGIN_REGEX "(\\S*)#"
> +#define VIR_STORAGE_VOL_LOGICAL_UUID_REGEX "(\\S+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_DEVICES_REGEX "(\\S+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_REGEX "(\\S+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_STRIPES_REGEX "([0-9]+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_SEG_SIZE_REGEX "(\\S+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_VG_EXTENT_SIZE_REGEX "([0-9]+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_SIZE_REGEX "([0-9]+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_LV_ATTR_REGEX "(\\S+)#"
> +#define VIR_STORAGE_VOL_LOGICAL_SUFFIX_REGEX "?\\s*$"
> +
> +#define VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT 10
> +#define VIR_STORAGE_VOL_LOGICAL_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_PREFIX_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_LV_NAME_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_ORIGIN_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_UUID_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_DEVICES_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_SEGTYPE_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_STRIPES_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_SEG_SIZE_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_VG_EXTENT_SIZE_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_SIZE_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_LV_ATTR_REGEX \
> +   VIR_STORAGE_VOL_LOGICAL_SUFFIX_REGEX
> +
>  static int
>  virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
>  virStorageVolDefPtr vol)
> @@ -342,10 +370,10 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr 
> pool,
>   *striped, so "," is not a suitable separator either (rhbz 727474).
>   */
>  const char *regexes[] = {
> -   
> "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$"
> +VIR_STORAGE_VOL_LOGICAL_REGEX
>  };
>  int vars[] = {
> -10
> +VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT
>  };
>  int ret = -1;
>  virCommandPtr cmd;

Clever way to label the various chunks, I love it. Crucially, the original
regex and the one obtained by putting all the bits together match. (See
what I did there?)

ACK.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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