Re: [libvirt] [PATCH v2 0/5] Prevent losing IPv6 routes due to forwarding

[Yalan Zhang]

Hi Cédric,

您好 ：）
I'm sorry that I missed the mail.
But currently I can not reproduce it.
For the error by net-create, it is executed when I set accept_ra to 1.

I have just test on libvirt-3.2.0-4.el7.x86_64, the behavior changes, it
seems like there is no check for accept_ra before start a network with ipv6.

1. define and start a network with ipv6 settings
# virsh net-dumpxml default6

  default6
  c502d02c-fbd0-49d9-91e4-0fcf0ef159d0
  
  
  
  

  

  
  

  

  


# cat /proc/sys/net/ipv6/conf/enp0s25/accept_ra
1

# virsh net-start default6   => the network can start as well with
accept_ra=1
Network default6 started

It seems that the "virNetDevIPGetAcceptRA()" in patch  "network: check
accept_ra before enabling ipv6 forwarding" with commit 00d28a78 is not
executed when I start a network. Please help to check, Thank you.









Best Regards,
Yalan Zhang
IRC: yalzhang
Internal phone: 8389413

On Tue, Apr 18, 2017 at 5:54 PM, Cedric Bosdonnat 
wrote:

> Yalan 你好
>
> On Mon, 2017-04-17 at 17:30 +0800, Yalan Zhang wrote:
> > I have tested it, it works well. But the interface name will repeat 2
> times.
> > Please help to confirm this, and if below test for a single port host is
> enough?
> >
> > # cat /proc/sys/net/ipv6/conf/enp0s25/accept_ra
> > 1
> >
> > enable network default with ipv6 ip section
> >
> > # virsh net-start default
> > error: Failed to start network default
> > error: internal error: Check the host setup: enabling IPv6 forwarding
> with RA routes without accept_ra set to 2 is
> > likely to cause routes loss. Interfaces to look at: enp0s25, enp0s25
>
> Just to help me confirm my intuition: do you have several RA routes defined
> for the same device?
>
> > # echo 2 > /proc/sys/net/ipv6/conf/enp0s25/accept_ra
> >
> > # virsh net-start default
> > Network default started
> >
> > try create:
> >
> > # virsh net-create default.xml
> > error: Failed to create network from default.xml
> > error: internal error: Check the host setup: enabling IPv6 forwarding
> with RA routes without accept_ra set to 2 is
> > likely to cause routes loss. Interfaces to look at: enp0s25, enp0s25
>
> This one sounds weird: if the accept_ra is set to 2 as you report you did,
> you shouldn't get that error.
>
> --
> Cedric
>
> > On Wed, Mar 15, 2017 at 10:45 PM, Cédric Bosdonnat 
> wrote:
> > > Hi Laine, all,
> > >
> > > Here is the v2 of my series. The changes are:
> > >
> > >  * Add a commit to create a virNetDevGetName() function
> > >  * Fix Laine's comments
> > >
> > > Cédric Bosdonnat (5):
> > >   util: extract the request sending code from virNetlinkCommand()
> > >   util: add virNetlinkDumpCommand()
> > >   bridge_driver.c: more uses of SYSCTL_PATH
> > >   util: add virNetDevGetName() function
> > >   network: check accept_ra before enabling ipv6 forwarding
> > >
> > >  src/libvirt_private.syms|   3 +
> > >  src/network/bridge_driver.c |  25 ---
> > >  src/util/virnetdev.c|  19 ++
> > >  src/util/virnetdev.h|   2 +
> > >  src/util/virnetdevip.c  | 158 ++
> ++
> > >  src/util/virnetdevip.h  |   1 +
> > >  src/util/virnetlink.c   | 145 ++
> --
> > >  src/util/virnetlink.h   |   9 +++
> > >  8 files changed, 319 insertions(+), 43 deletions(-)
> > >
> > > --
> > > 2.11.0
> > >
> > > --
> > > libvir-list mailing list
> > > libvir-list@redhat.com
> > > https://www.redhat.com/mailman/listinfo/libvir-list
> >
> >
> >
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] libxl: report numa sibling distances on host capabilities

[Wim Ten Have]

From: Wim ten Have 

When running on a NUMA machine, populate the sibling node
and distance information using data supplied by Xen.

With locality distances information, under Xen, new host
capabilities would like:


  

  263902380
  


  
  ...

...
  
  ...


Signed-off-by: Wim ten Have 
---
 src/libxl/libxl_capabilities.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 839a2ee..e095920 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -247,8 +247,9 @@ libxlCapsInitNuma(libxl_ctx *ctx, virCapsPtr caps)
 {
 libxl_numainfo *numa_info = NULL;
 libxl_cputopology *cpu_topo = NULL;
-int nr_nodes = 0, nr_cpus = 0;
+int nr_nodes = 0, nr_cpus = 0, nr_siblings = 0;
 virCapsHostNUMACellCPUPtr *cpus = NULL;
+virCapsHostNUMACellSiblingInfoPtr siblings = NULL;
 int *nr_cpus_node = NULL;
 size_t i;
 int ret = -1;
@@ -322,10 +323,23 @@ libxlCapsInitNuma(libxl_ctx *ctx, virCapsPtr caps)
 if (numa_info[i].size == LIBXL_NUMAINFO_INVALID_ENTRY)
 continue;
 
+nr_siblings = numa_info[i].num_dists;
+if (nr_siblings) {
+size_t j;
+
+if (VIR_ALLOC_N(siblings, nr_siblings) < 0)
+goto cleanup;
+
+for (j = 0; j < nr_siblings; j++) {
+siblings[j].node = j;
+siblings[j].distance = numa_info[i].dists[j];
+}
+}
+
 if (virCapabilitiesAddHostNUMACell(caps, i,
numa_info[i].size / 1024,
nr_cpus_node[i], cpus[i],
-   0, NULL,
+   nr_siblings, siblings,
0, NULL) < 0) {
 virCapabilitiesClearHostNUMACellCPUTopology(cpus[i],
 nr_cpus_node[i]);
@@ -343,6 +357,7 @@ libxlCapsInitNuma(libxl_ctx *ctx, virCapsPtr caps)
 for (i = 0; cpus && i < nr_nodes; i++)
 VIR_FREE(cpus[i]);
 virCapabilitiesFreeNUMAInfo(caps);
+VIR_FREE(siblings);
 }
 
 VIR_FREE(cpus);
-- 
2.9.3

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

[libvirt] [PATCH v2] libxl: report numa sibling distances on host capabilities

[Wim Ten Have]

From: Wim ten Have 

When running on a NUMA machine, populate the sibling node
and distance information using data supplied by Xen.

Wim ten Have (1):
  libxl: report numa sibling distances on host capabilities

 src/libxl/libxl_capabilities.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

-- 
2.9.3

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

Re: [libvirt] [PATCH] Add "io" option to virsh attach-disk sub-command.

[Gordon Messmer]

On 05/09/2017 07:58 AM, Daniel P. Berrange wrote:

Opps, yes, mixed terminology. You're right - virsh is lacking this
feature. Feel free to send a patch to add a '--io' flag to the
attach-disk command if you want to try fixing this.



That's what my original message contained.  :)

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

Re: [libvirt] [PATCH] spec: Update version check for maint Source URL

[Cole Robinson]

On 05/09/2017 11:57 AM, Andrea Bolognani wrote:
> On Thu, 2017-05-04 at 20:08 -0400, Cole Robinson wrote:
>> New maint release version numbers of just A.B.C format, not the old
>> A.B.C.D format. Adjust the check that dynamically changes the Source
>> URL for maint releases
>> ---
>>   libvirt.spec.in | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> By the way, the machinery to deal with mainteinance release
> is missing altogether from mingw-libvirt.spec.in, shouldn't
> we add it there as well?

Yes I think so, I've just never done a mingw build so didn't consider it

Thanks,
Cole

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

Re: [libvirt] [PATCH 0/8] Multiple cleanups within interfaceobj and interface driver

[John Ferlan]

ping?

NB: In the virInterfaceObjList patch, I've added a
VIR_FREE(interfaces->objs); to virInterfaceObjListFree. It should be
obvious why...

Tks -

John

On 04/25/2017 06:36 PM, John Ferlan wrote:
> Sensing a theme lately yet?
> 
> More adjustments in preparation for having virobject code handle the bulk
> of the object management code.
> 
> Far less in this series than the other two... The Clone code was a bit
> unique and required more of a rewrite than a refigure since now we have
> two allocated lists rather than two static lists.
> 
> John Ferlan (8):
>   interface: Consistently use 'obj' for a virInterfaceObjPtr
>   interface: Remove some unnecessary goto's for Interface tests
>   interface: Use virInterfaceDefPtr rather than deref from
> virInterfaceObjPtr
>   interface: Make _virInterfaceObj struct private
>   interface: Make _virInterfaceObjList struct private
>   interface: Rename some virInterfaceObj* API's
>   interface: Clean up virInterfaceObjListFindByMACString
>   interface: Introduce virInterfaceObjNew
> 
>  src/conf/virinterfaceobj.c | 232 
> +
>  src/conf/virinterfaceobj.h |  65 ++---
>  src/libvirt_private.syms   |  15 +--
>  src/test/test_driver.c | 143 ++--
>  4 files changed, 262 insertions(+), 193 deletions(-)
> 

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

Re: [libvirt] [PATCH] conf: Resolve corner case on fc_host deletion

[John Ferlan]

ping?

Tks -

John

On 04/29/2017 11:36 AM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1420740
> 
> Testing found an inventive way to cause an error at shutdown by providing the
> parent name for the fc host creation using the "same name" as the HBA. Since
> the code thus assumed the parent host name provided was the parent HBA and
> just extracted out the host number and sent that along to the vport_destroy
> this avoided checks made for equality.
> 
> So just add the equality check to that path to resolve.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/node_device_conf.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 85cfd83..3f995da 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -2127,17 +2127,25 @@ virNodeDeviceDeleteVport(virConnectPtr conn,
>  goto cleanup;
>  }
>  
> +if (virAsprintf(_host_name, "scsi_%s", name) < 0)
> +goto cleanup;
> +
>  /* If at startup time we provided a parent, then use that to
>   * get the parent_host value; otherwise, we have to determine
>   * the parent scsi_host which we did not save at startup time
>   */
>  if (fchost->parent) {
> +/* Someone provided a parent string at startup time that
> + * was the same as the scsi_host - meaning we have a pool
> + * backed to an HBA, so there won't be a vHBA to delete */
> +if (STREQ(scsi_host_name, fchost->parent)) {
> +ret = 0;
> +goto cleanup;
> +}
> +
>  if (virSCSIHostGetNumber(fchost->parent, _host) < 0)
>  goto cleanup;
>  } else {
> -if (virAsprintf(_host_name, "scsi_%s", name) < 0)
> -goto cleanup;
> -
>  if (!(vhba_parent = virNodeDeviceGetParentName(conn, 
> scsi_host_name)))
>  goto cleanup;
>  
> 

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

Re: [libvirt] [PATCH] spec: Update version check for maint Source URL

[Andrea Bolognani]

On Thu, 2017-05-04 at 20:08 -0400, Cole Robinson wrote:
> New maint release version numbers of just A.B.C format, not the old
> A.B.C.D format. Adjust the check that dynamically changes the Source
> URL for maint releases
> ---
>  libvirt.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

By the way, the machinery to deal with mainteinance release
is missing altogether from mingw-libvirt.spec.in, shouldn't
we add it there as well?

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] spec: Update version check for maint Source URL

[Andrea Bolognani]

On Thu, 2017-05-04 at 20:08 -0400, Cole Robinson wrote:
> New maint release version numbers of just A.B.C format, not the old
> A.B.C.D format. Adjust the check that dynamically changes the Source
> URL for maint releases
> ---
>  libvirt.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] conf: Check CPU cache for ABI stability

[Daniel P. Berrange]

On Tue, May 09, 2017 at 05:44:14PM +0200, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  src/conf/cpu_conf.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index 1b098c476..194f03faf 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -915,6 +915,16 @@ virCPUDefIsEqual(virCPUDefPtr src,
>  }
>  }
>  
> +if ((src->cache && !dst->cache) ||
> +(!src->cache && dst->cache) ||
> +(src->cache && dst->cache &&
> + (src->cache->level != dst->cache->level ||
> +  src->cache->mode != dst->cache->mode))) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   "Target CPU cache does not match source");
> +goto cleanup;
> +}
> +
>  identical = true;
>  
>   cleanup:

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 2/3] Add support for CPU cache specification

[Jiri Denemark]

On Mon, May 08, 2017 at 15:37:24 +0100, Daniel P. Berrange wrote:
> On Tue, Apr 25, 2017 at 08:48:44PM +0200, Jiri Denemark wrote:
> > This patch introduces
> > 
> > 
> > 
> > 
> > 
> > sub element of /domain/cpu. Currently only a single  element is
> > allowed.
> > 
> > Signed-off-by: Jiri Denemark 
> 
> > ---
> >  docs/formatdomain.html.in  | 35 
> >  docs/schemas/cputypes.rng  | 21 
> >  docs/schemas/domaincommon.rng  |  3 ++
> >  src/conf/cpu_conf.c| 62 
> > ++
> 
> I could be mistake, but I didn't see any additions to cpu_conf.c
> to validate CPU cache compatibility. ie nothing seems to prevent
> you changing the cache XML when doing migration, which will lead
> to badness

Oh yeah, thanks for noticing this. I've just sent the fix:
https://www.redhat.com/archives/libvir-list/2017-May/msg00238.html

Jirka

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

[libvirt] [PATCH] conf: Check CPU cache for ABI stability

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/conf/cpu_conf.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 1b098c476..194f03faf 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -915,6 +915,16 @@ virCPUDefIsEqual(virCPUDefPtr src,
 }
 }
 
+if ((src->cache && !dst->cache) ||
+(!src->cache && dst->cache) ||
+(src->cache && dst->cache &&
+ (src->cache->level != dst->cache->level ||
+  src->cache->mode != dst->cache->mode))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   "Target CPU cache does not match source");
+goto cleanup;
+}
+
 identical = true;
 
  cleanup:
-- 
2.12.2

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

[libvirt] [PATCH 15/19] storage: Introduce virStoragePoolObjForEachVolume

[John Ferlan]

Create/Use API to "walk" the storage pool object volume list.

Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c   | 10 ++
 src/conf/virstorageobj.h   |  4 
 src/libvirt_private.syms   |  1 +
 src/storage/storage_backend_disk.c | 26 +++---
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index cc3464e..14feecb 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -343,6 +343,16 @@ virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj)
 
 
 virStorageVolDefPtr
+virStoragePoolObjForEachVolume(virStoragePoolObjPtr obj,
+   size_t curidx)
+{
+if (curidx < obj->volumes.count)
+return obj->volumes.objs[curidx];
+return NULL;
+}
+
+
+virStorageVolDefPtr
 virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
   const char *key)
 {
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index d27ff57..df0b4ae 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -139,6 +139,10 @@ size_t
 virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj);
 
 virStorageVolDefPtr
+virStoragePoolObjForEachVolume(virStoragePoolObjPtr obj,
+   size_t curidx);
+
+virStorageVolDefPtr
 virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
   const char *key);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 03777a3..fe0e203 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1014,6 +1014,7 @@ virStoragePoolObjDecrAsyncjobs;
 virStoragePoolObjDeleteDef;
 virStoragePoolObjFindByName;
 virStoragePoolObjFindByUUID;
+virStoragePoolObjForEachVolume;
 virStoragePoolObjGetAsyncjobs;
 virStoragePoolObjGetAutostart;
 virStoragePoolObjGetConfigFile;
diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index 0bf5567..0ec601e 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -192,12 +192,11 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr 
pool,
 /* Find the extended partition and increase the allocation value */
 if (vol->source.partType == VIR_STORAGE_VOL_DISK_TYPE_LOGICAL) {
 size_t i;
+virStorageVolDefPtr voldef;
 
-for (i = 0; i < pool->volumes.count; i++) {
-if (pool->volumes.objs[i]->source.partType ==
-VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) {
-pool->volumes.objs[i]->target.allocation +=
-vol->target.allocation;
+for (i = 0; (voldef = virStoragePoolObjForEachVolume(pool, i)); i++) {
+if (voldef->source.partType == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) 
{
+voldef->target.allocation += vol->target.allocation;
 break;
 }
 }
@@ -533,8 +532,10 @@ virStorageBackendDiskPartTypeToCreate(virStoragePoolObjPtr 
pool)
can't be more than 3 to create a new primary partition */
 size_t i;
 int count = 0;
-for (i = 0; i < pool->volumes.count; i++) {
-int partType = pool->volumes.objs[i]->source.partType;
+virStorageVolDefPtr voldef;
+
+for (i = 0; (voldef = virStoragePoolObjForEachVolume(pool, i)); i++) {
+int partType = voldef->source.partType;
 if (partType == VIR_STORAGE_VOL_DISK_TYPE_PRIMARY ||
 partType == VIR_STORAGE_VOL_DISK_TYPE_EXTENDED)
 count++;
@@ -553,6 +554,8 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
 char** partFormat)
 {
 size_t i;
+virStorageVolDefPtr voldef;
+
 if (pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) {
 const char *partedFormat;
 partedFormat = virStoragePartedFsTypeToString(vol->target.format);
@@ -563,8 +566,8 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
 }
 if (vol->target.format == VIR_STORAGE_VOL_DISK_EXTENDED) {
 /* make sure we don't have an extended partition already */
-for (i = 0; i < pool->volumes.count; i++) {
-if (pool->volumes.objs[i]->source.partType ==
+for (i = 0; (voldef = virStoragePoolObjForEachVolume(pool, i)); 
i++) {
+if (voldef->source.partType ==
 VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("extended partition already exists"));
@@ -585,8 +588,9 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
 break;
 case VIR_STORAGE_VOL_DISK_TYPE_LOGICAL:
 /* make sure we have an extended partition */
-for (i = 0; i < pool->volumes.count; i++) {
-if (pool->volumes.objs[i]->source.partType ==
+  

[libvirt] [PATCH 19/19] storage: Change storage_util to use obj instead of pool

[John Ferlan]

For consistency sake - use @obj as the variable name.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 106 ++---
 src/storage/storage_util.h |  30 ++---
 2 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 63ffc53..ad05cbf 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -223,7 +223,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 
 static int
 storageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED,
-  virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+  virStoragePoolObjPtr obj ATTRIBUTE_UNUSED,
   virStorageVolDefPtr vol,
   virStorageVolDefPtr inputvol,
   unsigned int flags)
@@ -391,12 +391,12 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 
 static int
 storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
-virStoragePoolObjPtr pool,
+virStoragePoolObjPtr obj,
 virStorageVolDefPtr vol,
 virStorageVolDefPtr inputvol,
 unsigned int flags)
 {
-virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
 int ret = -1;
 int fd = -1;
 int operation_flags;
@@ -594,11 +594,11 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
 }
 
 static int
-virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
+virStorageBackendCreateExecCommand(virStoragePoolObjPtr obj,
virStorageVolDefPtr vol,
virCommandPtr cmd)
 {
-virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
 struct stat st;
 gid_t gid;
 uid_t uid;
@@ -696,7 +696,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
pool,
  * if function fails to create image file the directory will be deleted.*/
 static int
 storageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED,
-  virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+  virStoragePoolObjPtr obj ATTRIBUTE_UNUSED,
   virStorageVolDefPtr vol,
   virStorageVolDefPtr inputvol,
   unsigned int flags)
@@ -1026,12 +1026,12 @@ storageBackendCreateQemuImgSetInput(virStorageVolDefPtr 
inputvol,
 
 
 static int
-storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
+storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr obj,
   virStorageVolDefPtr vol,
   virStorageVolDefPtr inputvol,
   struct _virStorageBackendQemuImgInfo 
*info)
 {
-virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
 int accessRetCode = -1;
 char *absolutePath = NULL;
 
@@ -1151,7 +1151,7 @@ storageBackendCreateQemuImgSecretObject(virCommandPtr cmd,
  */
 virCommandPtr
 virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
- virStoragePoolObjPtr pool,
+ virStoragePoolObjPtr obj,
  virStorageVolDefPtr vol,
  virStorageVolDefPtr inputvol,
  unsigned int flags,
@@ -1231,7 +1231,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr 
conn,
 return NULL;
 
 if (vol->target.backingStore &&
-storageBackendCreateQemuImgSetBacking(pool, vol, inputvol, ) < 0)
+storageBackendCreateQemuImgSetBacking(obj, vol, inputvol, ) < 0)
 return NULL;
 
 if (info.encryption &&
@@ -1288,7 +1288,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr 
conn,
 
 static char *
 storageBackendCreateQemuImgSecretPath(virConnectPtr conn,
-  virStoragePoolObjPtr pool,
+  virStoragePoolObjPtr obj,
   virStorageVolDefPtr vol)
 {
 virStorageEncryptionPtr enc = vol->target.encryption;
@@ -1312,7 +1312,7 @@ storageBackendCreateQemuImgSecretPath(virConnectPtr conn,
 return NULL;
 }
 
-if (!(secretPath = virStoragePoolObjBuildTempFilePath(pool, vol)))
+if (!(secretPath = virStoragePoolObjBuildTempFilePath(obj, vol)))
 goto cleanup;
 
 if ((fd = mkostemp(secretPath, O_CLOEXEC)) < 0) {
@@ -1358,7 +1358,7 @@ storageBackendCreateQemuImgSecretPath(virConnectPtr conn,
 
 static int
 storageBackendCreateQemuImg(virConnectPtr conn,
-virStoragePoolObjPtr 

[libvirt] [PATCH 18/19] storage: Use virStoragePoolObj accessors for storage_util

[John Ferlan]

In preparation for privatizing the object, use the accessors.

Perform some minor code cleanup along the w/r/t spacing, line wraps, etc.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 116 -
 1 file changed, 63 insertions(+), 53 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 8d9dd3f..63ffc53 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -396,6 +396,7 @@ storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
 virStorageVolDefPtr inputvol,
 unsigned int flags)
 {
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 int ret = -1;
 int fd = -1;
 int operation_flags;
@@ -431,7 +432,7 @@ storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
 }
 
 operation_flags = VIR_FILE_OPEN_FORCE_MODE | VIR_FILE_OPEN_FORCE_OWNER;
-if (pool->def->type == VIR_STORAGE_POOL_NETFS)
+if (def->type == VIR_STORAGE_POOL_NETFS)
 operation_flags |= VIR_FILE_OPEN_FORK;
 
 if (vol->target.perms->mode != (mode_t) -1)
@@ -597,6 +598,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
pool,
virStorageVolDefPtr vol,
virCommandPtr cmd)
 {
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 struct stat st;
 gid_t gid;
 uid_t uid;
@@ -606,7 +608,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
pool,
 bool filecreated = false;
 int ret = -1;
 
-if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
+if ((def->type == VIR_STORAGE_POOL_NETFS)
 && (((geteuid() == 0)
  && (vol->target.perms->uid != (uid_t) -1)
  && (vol->target.perms->uid != 0))
@@ -1029,6 +1031,7 @@ 
storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
   virStorageVolDefPtr inputvol,
   struct _virStorageBackendQemuImgInfo 
*info)
 {
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 int accessRetCode = -1;
 char *absolutePath = NULL;
 
@@ -1071,7 +1074,7 @@ 
storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
  * validation.
  */
 if ('/' != *(info->backingPath) &&
-virAsprintf(, "%s/%s", pool->def->target.path,
+virAsprintf(, "%s/%s", def->target.path,
 info->backingPath) < 0)
 return -1;
 accessRetCode = access(absolutePath ? absolutePath :
@@ -1921,7 +1924,7 @@ virStorageBackendPoolPathIsStable(const char *path)
 
 /*
  * Given a volume path directly in /dev/XXX, iterate over the
- * entries in the directory pool->def->target.path and find the
+ * entries in the directory def->target.path and find the
  * first symlink pointing to the volume path.
  *
  * If, the target.path is /dev/, then return the original volume
@@ -1940,6 +1943,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
 const char *devpath,
 bool loop)
 {
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 DIR *dh;
 struct dirent *dent;
 char *stablepath;
@@ -1948,8 +1952,8 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
 int direrr;
 
 /* Logical pools are under /dev but already have stable paths */
-if (pool->def->type == VIR_STORAGE_POOL_LOGICAL ||
-!virStorageBackendPoolPathIsStable(pool->def->target.path))
+if (def->type == VIR_STORAGE_POOL_LOGICAL ||
+!virStorageBackendPoolPathIsStable(def->target.path))
 goto ret_strdup;
 
 /* We loop here because /dev/disk/by-{id,path} may not have existed
@@ -1957,7 +1961,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
  * get created.
  */
  reopen:
-if (virDirOpenQuiet(, pool->def->target.path) < 0) {
+if (virDirOpenQuiet(, def->target.path) < 0) {
 opentries++;
 if (loop && errno == ENOENT && opentries < 50) {
 usleep(100 * 1000);
@@ -1965,7 +1969,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
 }
 virReportSystemError(errno,
  _("cannot read dir '%s'"),
- pool->def->target.path);
+ def->target.path);
 return NULL;
 }
 
@@ -1981,8 +1985,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
  retry:
 while ((direrr = virDirRead(dh, , NULL)) > 0) {
 if (virAsprintf(, "%s/%s",
-pool->def->target.path,
-dent->d_name) < 0) {
+def->target.path, dent->d_name) < 0) {
 VIR_DIR_CLOSE(dh);
 return NULL;
 }
@@ -2020,6 +2023,7 @@ createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
   virStorageVolDefPtr inputvol,
   unsigned int 

[libvirt] [PATCH 16/19] storage: Use virStoragePoolObj accessors for driver

[John Ferlan]

In preparation for privatizing the object, use the accessors.

Perform some minor code cleanup along the w/r/t spacing, line wraps, etc.

Signed-off-by: John Ferlan 
---
 src/storage/storage_driver.c | 426 ---
 1 file changed, 242 insertions(+), 184 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index fc393dd..7d491bf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -90,14 +90,14 @@ static void
 virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
 {
 virStoragePoolObjPtr obj = *objptr;
+virStoragePoolDefPtr newDef = virStoragePoolObjGetNewDef(obj);
+char *configFile = virStoragePoolObjGetConfigFile(obj);
 
-if (obj->configFile == NULL) {
+if (!configFile) {
 virStoragePoolObjRemove(>pools, obj);
 *objptr = NULL;
-} else if (obj->newDef) {
-virStoragePoolDefFree(obj->def);
-obj->def = obj->newDef;
-obj->newDef = NULL;
+} else if (newDef) {
+virStoragePoolObjStealNewDef(obj);
 }
 }
 
@@ -105,17 +105,17 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
 static void
 storagePoolUpdateState(virStoragePoolObjPtr obj)
 {
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
 bool active = false;
 virStorageBackendPtr backend;
 char *stateFile;
 
-if (!(stateFile = virFileBuildPath(driver->stateDir,
-   obj->def->name, ".xml")))
+if (!(stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml")))
 goto cleanup;
 
-if ((backend = virStorageBackendForType(obj->def->type)) == NULL) {
+if ((backend = virStorageBackendForType(def->type)) == NULL) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Missing backend %d"), obj->def->type);
+   _("Missing backend %d"), def->type);
 goto cleanup;
 }
 
@@ -125,7 +125,7 @@ storagePoolUpdateState(virStoragePoolObjPtr obj)
 backend->checkPool(obj, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to initialize storage pool '%s': %s"),
-   obj->def->name, virGetLastErrorMessage());
+   def->name, virGetLastErrorMessage());
 active = false;
 }
 
@@ -140,14 +140,14 @@ storagePoolUpdateState(virStoragePoolObjPtr obj)
 backend->stopPool(NULL, obj);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to restart storage pool '%s': %s"),
-   obj->def->name, virGetLastErrorMessage());
+   def->name, virGetLastErrorMessage());
 active = false;
 }
 }
 
-obj->active = active;
+virStoragePoolObjSetActive(obj, active);
 
-if (!obj->active)
+if (!virStoragePoolObjIsActive(obj))
 virStoragePoolUpdateInactive();
 
  cleanup:
@@ -187,22 +187,23 @@ storageDriverAutostart(void)
 
 for (i = 0; i < driver->pools.count; i++) {
 virStoragePoolObjPtr obj = driver->pools.objs[i];
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
 virStorageBackendPtr backend;
 bool started = false;
 
 virStoragePoolObjLock(obj);
-if ((backend = virStorageBackendForType(obj->def->type)) == NULL) {
+if ((backend = virStorageBackendForType(def->type)) == NULL) {
 virStoragePoolObjUnlock(obj);
 continue;
 }
 
-if (obj->autostart &&
+if (virStoragePoolObjGetAutostart(obj) &&
 !virStoragePoolObjIsActive(obj)) {
 if (backend->startPool &&
 backend->startPool(conn, obj) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to autostart storage pool '%s': %s"),
-   obj->def->name, virGetLastErrorMessage());
+   def->name, virGetLastErrorMessage());
 virStoragePoolObjUnlock(obj);
 continue;
 }
@@ -213,10 +214,9 @@ storageDriverAutostart(void)
 char *stateFile;
 
 virStoragePoolObjClearVols(obj);
-stateFile = virFileBuildPath(driver->stateDir,
- obj->def->name, ".xml");
+stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml");
 if (!stateFile ||
-virStoragePoolSaveState(stateFile, obj->def) < 0 ||
+virStoragePoolSaveState(stateFile, def) < 0 ||
 backend->refreshPool(conn, obj) < 0) {
 if (stateFile)
 unlink(stateFile);
@@ -224,9 +224,9 @@ storageDriverAutostart(void)
 backend->stopPool(conn, obj);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to autostart 

[libvirt] [PATCH 13/19] storage: Move autostartLink deletion to virstorageobj

[John Ferlan]

The virStoragePoolObjDeleteDef already dealt with the configFile - just
add in the autostartLink as well. If there is no autostartLink defined,
then no need to attempt unlink - which also allows removal of one errno
check in moved function.

Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c | 11 +++
 src/storage/storage_driver.c | 11 ---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 9ce3840..69ed66d 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -696,6 +696,17 @@ virStoragePoolObjDeleteDef(virStoragePoolObjPtr obj)
obj->def->name);
 return -1;
 }
+VIR_FREE(obj->configFile);
+
+if (!obj->autostartLink)
+return 0;
+
+if (unlink(obj->autostartLink) < 0 && errno != ENOTDIR) {
+char ebuf[1024];
+VIR_ERROR(_("Failed to delete autostart link '%s': %s"),
+  obj->autostartLink, virStrerror(errno, ebuf, sizeof(ebuf)));
+}
+VIR_FREE(obj->autostartLink);
 
 return 0;
 }
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c4e4e7b..c4650cd 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -841,17 +841,6 @@ storagePoolUndefine(virStoragePoolPtr pool)
 if (virStoragePoolObjDeleteDef(obj) < 0)
 goto cleanup;
 
-if (unlink(obj->autostartLink) < 0 &&
-errno != ENOENT &&
-errno != ENOTDIR) {
-char ebuf[1024];
-VIR_ERROR(_("Failed to delete autostart link '%s': %s"),
-  obj->autostartLink, virStrerror(errno, ebuf, sizeof(ebuf)));
-}
-
-VIR_FREE(obj->configFile);
-VIR_FREE(obj->autostartLink);
-
 event = virStoragePoolEventLifecycleNew(obj->def->name,
 obj->def->uuid,
 VIR_STORAGE_POOL_EVENT_UNDEFINED,
-- 
2.9.3

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

[libvirt] [PATCH 07/19] storage: Use consistent variable names in virstorageobj

[John Ferlan]

A virStoragePoolObjPtr will be an 'obj'.

A virStoragePoolPtr will be a 'pool'.

NB: Also modify the @matchpool to @matchobj.
Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c | 342 +++
 src/conf/virstorageobj.h |  20 +--
 2 files changed, 180 insertions(+), 182 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index dd41701..74a9c67 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -70,15 +70,15 @@ virStoragePoolObjListFree(virStoragePoolObjListPtr pools)
 
 void
 virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
-virStoragePoolObjPtr pool)
+virStoragePoolObjPtr obj)
 {
 size_t i;
 
-virStoragePoolObjUnlock(pool);
+virStoragePoolObjUnlock(obj);
 
 for (i = 0; i < pools->count; i++) {
 virStoragePoolObjLock(pools->objs[i]);
-if (pools->objs[i] == pool) {
+if (pools->objs[i] == obj) {
 virStoragePoolObjUnlock(pools->objs[i]);
 virStoragePoolObjFree(pools->objs[i]);
 
@@ -125,15 +125,15 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr 
pools,
 
 
 static virStoragePoolObjPtr
-virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool,
+virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr obj,
  virStoragePoolDefPtr def)
 {
 size_t i, j;
 
-for (i = 0; i < pool->def->source.ndevice; i++) {
+for (i = 0; i < obj->def->source.ndevice; i++) {
 for (j = 0; j < def->source.ndevice; j++) {
-if (STREQ(pool->def->source.devices[i].path, 
def->source.devices[j].path))
-return pool;
+if (STREQ(obj->def->source.devices[i].path, 
def->source.devices[j].path))
+return obj;
 }
 }
 
@@ -142,54 +142,54 @@ 
virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool,
 
 
 void
-virStoragePoolObjClearVols(virStoragePoolObjPtr pool)
+virStoragePoolObjClearVols(virStoragePoolObjPtr obj)
 {
 size_t i;
-for (i = 0; i < pool->volumes.count; i++)
-virStorageVolDefFree(pool->volumes.objs[i]);
+for (i = 0; i < obj->volumes.count; i++)
+virStorageVolDefFree(obj->volumes.objs[i]);
 
-VIR_FREE(pool->volumes.objs);
-pool->volumes.count = 0;
+VIR_FREE(obj->volumes.objs);
+obj->volumes.count = 0;
 }
 
 
 virStorageVolDefPtr
-virStorageVolDefFindByKey(virStoragePoolObjPtr pool,
+virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
   const char *key)
 {
 size_t i;
 
-for (i = 0; i < pool->volumes.count; i++)
-if (STREQ(pool->volumes.objs[i]->key, key))
-return pool->volumes.objs[i];
+for (i = 0; i < obj->volumes.count; i++)
+if (STREQ(obj->volumes.objs[i]->key, key))
+return obj->volumes.objs[i];
 
 return NULL;
 }
 
 
 virStorageVolDefPtr
-virStorageVolDefFindByPath(virStoragePoolObjPtr pool,
+virStorageVolDefFindByPath(virStoragePoolObjPtr obj,
const char *path)
 {
 size_t i;
 
-for (i = 0; i < pool->volumes.count; i++)
-if (STREQ(pool->volumes.objs[i]->target.path, path))
-return pool->volumes.objs[i];
+for (i = 0; i < obj->volumes.count; i++)
+if (STREQ(obj->volumes.objs[i]->target.path, path))
+return obj->volumes.objs[i];
 
 return NULL;
 }
 
 
 virStorageVolDefPtr
-virStorageVolDefFindByName(virStoragePoolObjPtr pool,
+virStorageVolDefFindByName(virStoragePoolObjPtr obj,
const char *name)
 {
 size_t i;
 
-for (i = 0; i < pool->volumes.count; i++)
-if (STREQ(pool->volumes.objs[i]->name, name))
-return pool->volumes.objs[i];
+for (i = 0; i < obj->volumes.count; i++)
+if (STREQ(obj->volumes.objs[i]->name, name))
+return obj->volumes.objs[i];
 
 return NULL;
 }
@@ -296,39 +296,39 @@ virStoragePoolObjPtr
 virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
 {
-virStoragePoolObjPtr pool;
+virStoragePoolObjPtr obj;
 
-if ((pool = virStoragePoolObjFindByName(pools, def->name))) {
-if (!virStoragePoolObjIsActive(pool)) {
-virStoragePoolDefFree(pool->def);
-pool->def = def;
+if ((obj = virStoragePoolObjFindByName(pools, def->name))) {
+if (!virStoragePoolObjIsActive(obj)) {
+virStoragePoolDefFree(obj->def);
+obj->def = def;
 } else {
-virStoragePoolDefFree(pool->newDef);
-pool->newDef = def;
+virStoragePoolDefFree(obj->newDef);
+obj->newDef = def;
 }
-return pool;
+return obj;
 }
 
-if (VIR_ALLOC(pool) < 0)
+if (VIR_ALLOC(obj) < 0)
 return NULL;
 
-if (virMutexInit(>lock) < 0) {
+if (virMutexInit(>lock) < 0) {
 

[libvirt] [PATCH 17/19] storage: Use virStoragePoolObj accessors for storage test API's

[John Ferlan]

In preparation for privatizing the object, use the accessors.

Perform some minor code cleanup along the w/r/t spacing, line wraps, etc.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 186 -
 1 file changed, 107 insertions(+), 79 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6ea3a5b..af5955d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1042,6 +1042,7 @@ testOpenVolumesForPool(const char *file,
virStoragePoolObjPtr obj,
int objidx)
 {
+virStoragePoolDefPtr objdef = virStoragePoolObjGetDef(obj);
 char *vol_xpath;
 size_t i;
 int num, ret = -1;
@@ -1063,13 +1064,13 @@ testOpenVolumesForPool(const char *file,
 if (!node)
 goto error;
 
-def = virStorageVolDefParseNode(obj->def, ctxt->doc, node, 0);
+def = virStorageVolDefParseNode(objdef, ctxt->doc, node, 0);
 if (!def)
 goto error;
 
 if (def->target.path == NULL) {
 if (virAsprintf(>target.path, "%s/%s",
-obj->def->target.path, def->name) < 0)
+objdef->target.path, def->name) < 0)
 goto error;
 }
 
@@ -1079,8 +1080,8 @@ testOpenVolumesForPool(const char *file,
 if (virStoragePoolObjAddVol(obj, def) < 0)
 goto error;
 
-obj->def->allocation += def->target.allocation;
-obj->def->available = (obj->def->capacity - obj->def->allocation);
+objdef->allocation += def->target.allocation;
+objdef->available = (objdef->capacity - objdef->allocation);
 def = NULL;
 }
 
@@ -1127,7 +1128,7 @@ testParseStorage(testDriverPtr privconn,
 virStoragePoolObjUnlock(obj);
 goto error;
 }
-obj->active = 1;
+virStoragePoolObjSetActive(obj, true);
 
 /* Find storage volumes */
 if (testOpenVolumesForPool(file, ctxt, obj, i+1) < 0) {
@@ -4001,12 +4002,18 @@ testInterfaceDestroy(virInterfacePtr iface,
 static int
 testStoragePoolObjSetDefaults(virStoragePoolObjPtr obj)
 {
+char *configFile;
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
+
+def->capacity = defaultPoolCap;
+def->allocation = defaultPoolAlloc;
+def->available = defaultPoolCap - defaultPoolAlloc;
 
-obj->def->capacity = defaultPoolCap;
-obj->def->allocation = defaultPoolAlloc;
-obj->def->available = defaultPoolCap - defaultPoolAlloc;
+if (VIR_STRDUP(configFile, "") < 0)
+return -1;
 
-return VIR_STRDUP(obj->configFile, "");
+virStoragePoolObjSetConfigFile(obj, configFile);
+return 0;
 }
 
 
@@ -4097,13 +4104,14 @@ testStoragePoolLookupByUUID(virConnectPtr conn,
 {
 testDriverPtr privconn = conn->privateData;
 virStoragePoolObjPtr obj;
+virStoragePoolDefPtr def;
 virStoragePoolPtr ret = NULL;
 
 if (!(obj = testStoragePoolObjFindByUUID(privconn, uuid)))
 return NULL;
+def = virStoragePoolObjGetDef(obj);
 
-ret = virGetStoragePool(conn, obj->def->name, obj->def->uuid,
-NULL, NULL);
+ret = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
 
 virStoragePoolObjUnlock(obj);
 return ret;
@@ -4116,13 +4124,14 @@ testStoragePoolLookupByName(virConnectPtr conn,
 {
 testDriverPtr privconn = conn->privateData;
 virStoragePoolObjPtr obj;
+virStoragePoolDefPtr def;
 virStoragePoolPtr ret = NULL;
 
 if (!(obj = testStoragePoolObjFindByName(privconn, name)))
 return NULL;
+def = virStoragePoolObjGetDef(obj);
 
-ret = virGetStoragePool(conn, obj->def->name, obj->def->uuid,
-NULL, NULL);
+ret = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
 
 virStoragePoolObjUnlock(obj);
 return ret;
@@ -4248,7 +4257,7 @@ testStoragePoolIsPersistent(virStoragePoolPtr pool)
 if (!(obj = testStoragePoolObjFindByUUID(privconn, pool->uuid)))
 return -1;
 
-ret = obj->configFile ? 1 : 0;
+ret = virStoragePoolObjGetConfigFile(obj) ? 1 : 0;
 
 virStoragePoolObjUnlock(obj);
 return ret;
@@ -4268,7 +4277,7 @@ testStoragePoolCreate(virStoragePoolPtr pool,
 if (!(obj = testStoragePoolObjFindInactiveByName(privconn, pool->name)))
 return -1;
 
-obj->active = 1;
+virStoragePoolObjSetActive(obj, true);
 
 event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
 VIR_STORAGE_POOL_EVENT_STARTED,
@@ -4369,6 +4378,8 @@ testStoragePoolCreateXML(virConnectPtr conn,
 testDriverPtr privconn = conn->privateData;
 virStoragePoolDefPtr def;
 virStoragePoolObjPtr obj = NULL;
+virStoragePoolDefPtr objdef;
+char *configFile;
 virStoragePoolPtr ret = NULL;
 virObjectEventPtr event = NULL;
 
@@ -4390,15 +4401,16 @@ 

[libvirt] [PATCH 11/19] storage: Introduce virStoragePoolObjNew

[John Ferlan]

Create/use a helper to perform object allocation.

Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c | 34 +++---
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 7d6b311..0079472 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -37,6 +37,27 @@
 VIR_LOG_INIT("conf.virstorageobj");
 
 
+static virStoragePoolObjPtr
+virStoragePoolObjNew(virStoragePoolDefPtr def)
+{
+virStoragePoolObjPtr obj;
+
+if (VIR_ALLOC(obj) < 0)
+return NULL;
+
+if (virMutexInit(>lock) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("cannot initialize mutex"));
+VIR_FREE(obj);
+return NULL;
+}
+virStoragePoolObjLock(obj);
+obj->active = 0;
+obj->def = def;
+return obj;
+}
+
+
 virStoragePoolDefPtr
 virStoragePoolObjGetDef(virStoragePoolObjPtr obj)
 {
@@ -386,24 +407,15 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
 return obj;
 }
 
-if (VIR_ALLOC(obj) < 0)
+if (!(obj = virStoragePoolObjNew(def)))
 return NULL;
 
-if (virMutexInit(>lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("cannot initialize mutex"));
-VIR_FREE(obj);
-return NULL;
-}
-virStoragePoolObjLock(obj);
-obj->active = 0;
-
 if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, obj) < 0) {
+obj->def = NULL;
 virStoragePoolObjUnlock(obj);
 virStoragePoolObjFree(obj);
 return NULL;
 }
-obj->def = def;
 
 return obj;
 }
-- 
2.9.3

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

[libvirt] [PATCH 12/19] storage: Introduce virStoragePoolObj{Get|Set}Autostart

[John Ferlan]

Rather than have the logic in the storage driver, move it to virstorageobj.

NB: virStoragePoolObjGetAutostart can take liberties with not needing the
same if...else construct.  Also pass a NULL for testStoragePoolSetAutostart
to avoid the autostartLink setup using the autostartDir for the test driver.

Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c | 57 
 src/conf/virstorageobj.h |  8 +++
 src/libvirt_private.syms |  2 ++
 src/storage/storage_driver.c | 41 +++
 src/test/test_driver.c   | 13 ++
 5 files changed, 72 insertions(+), 49 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 0079472..9ce3840 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -111,6 +111,63 @@ virStoragePoolObjSetActive(virStoragePoolObjPtr obj,
 }
 
 
+int
+virStoragePoolObjGetAutostart(virStoragePoolObjPtr obj)
+{
+if (!obj->configFile)
+return 0;
+
+return obj->autostart;
+}
+
+
+int
+virStoragePoolObjSetAutostart(virStoragePoolObjPtr obj,
+  const char *autostartDir,
+  int autostart)
+{
+obj->autostart = autostart;
+
+if (!obj->configFile) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("pool has no config file"));
+return -1;
+}
+
+autostart = (autostart != 0);
+
+if (obj->autostart != autostart) {
+if (autostart && autostartDir) {
+if (virFileMakePath(autostartDir) < 0) {
+virReportSystemError(errno,
+ _("cannot create autostart directory %s"),
+ autostartDir);
+return -1;
+}
+
+if (symlink(obj->configFile, obj->autostartLink) < 0) {
+virReportSystemError(errno,
+ _("Failed to create symlink '%s' to 
'%s'"),
+ obj->autostartLink, obj->configFile);
+return -1;
+}
+} else {
+if (unlink(obj->autostartLink) < 0 &&
+errno != ENOENT && errno != ENOTDIR) {
+virReportSystemError(errno,
+ _("Failed to delete symlink '%s'"),
+ obj->autostartLink);
+return -1;
+}
+}
+
+obj->autostart = autostart;
+}
+
+return 0;
+}
+
+
 unsigned int
 virStoragePoolObjGetAsyncjobs(virStoragePoolObjPtr obj)
 {
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index d47b233..3b6e395 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -93,6 +93,14 @@ void
 virStoragePoolObjSetActive(virStoragePoolObjPtr obj,
bool active);
 
+int
+virStoragePoolObjGetAutostart(virStoragePoolObjPtr obj);
+
+int
+virStoragePoolObjSetAutostart(virStoragePoolObjPtr obj,
+  const char *autostartDir,
+  int autostart);
+
 unsigned int
 virStoragePoolObjGetAsyncjobs(virStoragePoolObjPtr obj);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index edd3174..e8b4eda 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1014,6 +1014,7 @@ virStoragePoolObjDeleteDef;
 virStoragePoolObjFindByName;
 virStoragePoolObjFindByUUID;
 virStoragePoolObjGetAsyncjobs;
+virStoragePoolObjGetAutostart;
 virStoragePoolObjGetConfigFile;
 virStoragePoolObjGetDef;
 virStoragePoolObjGetNames;
@@ -1031,6 +1032,7 @@ virStoragePoolObjNumOfVolumes;
 virStoragePoolObjRemove;
 virStoragePoolObjSaveDef;
 virStoragePoolObjSetActive;
+virStoragePoolObjSetAutostart;
 virStoragePoolObjSetConfigFile;
 virStoragePoolObjSourceFindDuplicate;
 virStoragePoolObjStealNewDef;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6289314..c4e4e7b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1250,11 +1250,7 @@ storagePoolGetAutostart(virStoragePoolPtr pool,
 if (virStoragePoolGetAutostartEnsureACL(pool->conn, obj->def) < 0)
 goto cleanup;
 
-if (!obj->configFile) {
-*autostart = 0;
-} else {
-*autostart = obj->autostart;
-}
+*autostart = virStoragePoolObjGetAutostart(obj);
 
 ret = 0;
 
@@ -1277,39 +1273,8 @@ storagePoolSetAutostart(virStoragePoolPtr pool,
 if (virStoragePoolSetAutostartEnsureACL(pool->conn, obj->def) < 0)
 goto cleanup;
 
-if (!obj->configFile) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("pool has no config file"));
-}
-
-autostart = (autostart != 0);
-
-if (obj->autostart != autostart) {
-if (autostart) {
-if (virFileMakePath(driver->autostartDir) < 0) {
-virReportSystemError(errno,
-   

[libvirt] [PATCH 09/19] storage: Alter volume num, name, and export API's to just take obj

[John Ferlan]

Alter the virStoragePoolObjNumOfVolumes, virStoragePoolObjVolumeGetNames,
and virStoragePoolObjVolumeListExport APIs to take a virStoragePoolObjPtr
instead of the >volumes and obj->def.

Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c | 15 +--
 src/conf/virstorageobj.h |  9 +++--
 src/storage/storage_driver.c |  7 +++
 src/test/test_driver.c   |  9 +++--
 4 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 74a9c67..23346f3 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -196,11 +196,12 @@ virStorageVolDefFindByName(virStoragePoolObjPtr obj,
 
 
 int
-virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
+virStoragePoolObjNumOfVolumes(virStoragePoolObjPtr obj,
   virConnectPtr conn,
-  virStoragePoolDefPtr pooldef,
   virStoragePoolVolumeACLFilter aclfilter)
 {
+virStoragePoolDefPtr pooldef = obj->def;
+virStorageVolDefListPtr volumes = >volumes;
 int nvolumes = 0;
 size_t i;
 
@@ -216,13 +217,14 @@ virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr 
volumes,
 
 
 int
-virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+virStoragePoolObjVolumeGetNames(virStoragePoolObjPtr obj,
 virConnectPtr conn,
-virStoragePoolDefPtr pooldef,
 virStoragePoolVolumeACLFilter aclfilter,
 char **const names,
 int maxnames)
 {
+virStoragePoolDefPtr pooldef = obj->def;
+virStorageVolDefListPtr volumes = >volumes;
 int nnames = 0;
 size_t i;
 
@@ -247,11 +249,12 @@ virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr 
volumes,
 
 int
 virStoragePoolObjVolumeListExport(virConnectPtr conn,
-  virStorageVolDefListPtr volumes,
-  virStoragePoolDefPtr pooldef,
+  virStoragePoolObjPtr obj,
   virStorageVolPtr **vols,
   virStoragePoolVolumeACLFilter aclfilter)
 {
+virStoragePoolDefPtr pooldef = obj->def;
+virStorageVolDefListPtr volumes = >volumes;
 int ret = -1;
 size_t i;
 virStorageVolPtr *tmp_vols = NULL;
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 494b888..5a61b2a 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -114,23 +114,20 @@ typedef bool
  virStorageVolDefPtr def);
 
 int
-virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
+virStoragePoolObjNumOfVolumes(virStoragePoolObjPtr obj,
   virConnectPtr conn,
-  virStoragePoolDefPtr pooldef,
   virStoragePoolVolumeACLFilter aclfilter);
 
 int
-virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+virStoragePoolObjVolumeGetNames(virStoragePoolObjPtr obj,
 virConnectPtr conn,
-virStoragePoolDefPtr pooldef,
 virStoragePoolVolumeACLFilter aclfilter,
 char **const names,
 int maxnames);
 
 int
 virStoragePoolObjVolumeListExport(virConnectPtr conn,
-  virStorageVolDefListPtr volumes,
-  virStoragePoolDefPtr pooldef,
+  virStoragePoolObjPtr obj,
   virStorageVolPtr **vols,
   virStoragePoolVolumeACLFilter aclfilter);
 
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6122396..6289314 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1339,7 +1339,7 @@ storagePoolNumOfVolumes(virStoragePoolPtr pool)
 goto cleanup;
 }
 
-ret = virStoragePoolObjNumOfVolumes(>volumes, pool->conn, obj->def,
+ret = virStoragePoolObjNumOfVolumes(obj, pool->conn,
 virStoragePoolNumOfVolumesCheckACL);
 
  cleanup:
@@ -1368,7 +1368,7 @@ storagePoolListVolumes(virStoragePoolPtr pool,
 goto cleanup;
 }
 
-n = virStoragePoolObjVolumeGetNames(>volumes, pool->conn, obj->def,
+n = virStoragePoolObjVolumeGetNames(obj, pool->conn,
 virStoragePoolListVolumesCheckACL,
 names, maxnames);
  cleanup:
@@ -1399,8 +1399,7 @@ storagePoolListAllVolumes(virStoragePoolPtr pool,
 goto cleanup;
 }
 
-ret = virStoragePoolObjVolumeListExport(pool->conn, >volumes,
-obj->def, vols,
+ret = 

[libvirt] [PATCH 04/19] test: Add helpers to fetch active/inactive storage pool by name

[John Ferlan]

Rather than have repetitive code - create/use a couple of helpers:

testStoragePoolObjFindActiveByName and testStoragePoolObjFindInactiveByName

This will also allow for the reduction of some cleanup path logic.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 256 +
 1 file changed, 86 insertions(+), 170 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index efa54ff..9918df6 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4030,6 +4030,46 @@ testStoragePoolObjFindByName(testDriverPtr privconn,
 
 
 static virStoragePoolObjPtr
+testStoragePoolObjFindActiveByName(testDriverPtr privconn,
+   const char *name)
+{
+virStoragePoolObjPtr obj;
+
+if (!(obj = testStoragePoolObjFindByName(privconn, name)))
+return NULL;
+
+if (!virStoragePoolObjIsActive(obj)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("storage pool '%s' is not active"), name);
+virStoragePoolObjUnlock(obj);
+return NULL;
+}
+
+return obj;
+}
+
+
+static virStoragePoolObjPtr
+testStoragePoolObjFindInactiveByName(testDriverPtr privconn,
+ const char *name)
+{
+virStoragePoolObjPtr obj;
+
+if (!(obj = testStoragePoolObjFindByName(privconn, name)))
+return NULL;
+
+if (virStoragePoolObjIsActive(obj)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("storage pool '%s' is already active"), name);
+virStoragePoolObjUnlock(obj);
+return NULL;
+}
+
+return obj;
+}
+
+
+static virStoragePoolObjPtr
 testStoragePoolObjFindByUUID(testDriverPtr privconn,
  const unsigned char *uuid)
 {
@@ -4221,32 +4261,22 @@ testStoragePoolCreate(virStoragePoolPtr pool,
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
-int ret = -1;
 virObjectEventPtr event = NULL;
 
 virCheckFlags(0, -1);
 
-if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
-
-if (virStoragePoolObjIsActive(obj)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("storage pool '%s' is already active"), pool->name);
-goto cleanup;
-}
+if (!(obj = testStoragePoolObjFindInactiveByName(privconn, pool->name)))
+return -1;
 
 obj->active = 1;
 
 event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
 VIR_STORAGE_POOL_EVENT_STARTED,
 0);
-ret = 0;
 
- cleanup:
 testObjectEventQueue(privconn, event);
-if (obj)
-virStoragePoolObjUnlock(obj);
-return ret;
+virStoragePoolObjUnlock(obj);
+return 0;
 }
 
 
@@ -4458,31 +4488,19 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
-int ret = -1;
 virObjectEventPtr event = NULL;
 
-if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
-
-if (virStoragePoolObjIsActive(obj)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("storage pool '%s' is already active"), pool->name);
-goto cleanup;
-}
+if (!(obj = testStoragePoolObjFindInactiveByName(privconn, pool->name)))
+return -1;
 
 event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid,
 VIR_STORAGE_POOL_EVENT_UNDEFINED,
 0);
 
 virStoragePoolObjRemove(>pools, obj);
-obj = NULL;
-ret = 0;
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
 testObjectEventQueue(privconn, event);
-return ret;
+return 0;
 }
 
 
@@ -4492,24 +4510,14 @@ testStoragePoolBuild(virStoragePoolPtr pool,
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
-int ret = -1;
 
 virCheckFlags(0, -1);
 
-if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
-
-if (virStoragePoolObjIsActive(obj)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("storage pool '%s' is already active"), pool->name);
-goto cleanup;
-}
-ret = 0;
+if (!(obj = testStoragePoolObjFindInactiveByName(privconn, pool->name)))
+return -1;
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
-return ret;
+virStoragePoolObjUnlock(obj);
+return 0;
 }
 
 
@@ -4559,14 +4567,8 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
 int ret = -1;
 virObjectEventPtr event = NULL;
 
-if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
-
-if (!virStoragePoolObjIsActive(obj)) {
-

[libvirt] [PATCH 10/19] storage: Create accessor API's for virStoragePoolObj

[John Ferlan]

In preparation for making a private object, create accessor API's for
consumer storage functions to use:

virStoragePoolObjGetDef
virStoragePoolObjGetNewDef
virStoragePoolObjStealNewDef
virStoragePoolObjGetConfigFile
virStoragePoolObjSetConfigFile
virStoragePoolObjIsActive
virStoragePoolObjSetActive
virStoragePoolObjGetAsyncjobs
virStoragePoolObjIncrAsyncjobs
virStoragePoolObjDecrAsyncjobs

Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c | 74 
 src/conf/virstorageobj.h | 36 +++
 src/libvirt_private.syms | 10 +++
 3 files changed, 115 insertions(+), 5 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 23346f3..7d6b311 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -37,6 +37,80 @@
 VIR_LOG_INIT("conf.virstorageobj");
 
 
+virStoragePoolDefPtr
+virStoragePoolObjGetDef(virStoragePoolObjPtr obj)
+{
+return obj->def;
+}
+
+
+virStoragePoolDefPtr
+virStoragePoolObjGetNewDef(virStoragePoolObjPtr obj)
+{
+return obj->newDef;
+}
+
+
+void
+virStoragePoolObjStealNewDef(virStoragePoolObjPtr obj)
+{
+virStoragePoolDefFree(obj->def);
+obj->def = obj->newDef;
+obj->newDef = NULL;
+}
+
+
+char *
+virStoragePoolObjGetConfigFile(virStoragePoolObjPtr obj)
+{
+return obj->configFile;
+}
+
+
+void
+virStoragePoolObjSetConfigFile(virStoragePoolObjPtr obj,
+   char *configFile)
+{
+obj->configFile = configFile;
+}
+
+
+bool
+virStoragePoolObjIsActive(virStoragePoolObjPtr obj)
+{
+return obj->active;
+}
+
+
+void
+virStoragePoolObjSetActive(virStoragePoolObjPtr obj,
+   bool active)
+{
+obj->active = active;
+}
+
+
+unsigned int
+virStoragePoolObjGetAsyncjobs(virStoragePoolObjPtr obj)
+{
+return obj->asyncjobs;
+}
+
+
+void
+virStoragePoolObjIncrAsyncjobs(virStoragePoolObjPtr obj)
+{
+obj->asyncjobs++;
+}
+
+
+void
+virStoragePoolObjDecrAsyncjobs(virStoragePoolObjPtr obj)
+{
+obj->asyncjobs--;
+}
+
+
 void
 virStoragePoolObjFree(virStoragePoolObjPtr obj)
 {
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 5a61b2a..d47b233 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -70,11 +70,37 @@ typedef bool
 (*virStoragePoolObjListFilter)(virConnectPtr conn,
virStoragePoolDefPtr def);
 
-static inline int
-virStoragePoolObjIsActive(virStoragePoolObjPtr obj)
-{
-return obj->active;
-}
+virStoragePoolDefPtr
+virStoragePoolObjGetDef(virStoragePoolObjPtr obj);
+
+virStoragePoolDefPtr
+virStoragePoolObjGetNewDef(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjStealNewDef(virStoragePoolObjPtr obj);
+
+char *
+virStoragePoolObjGetConfigFile(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjSetConfigFile(virStoragePoolObjPtr obj,
+   char *configFile);
+
+bool
+virStoragePoolObjIsActive(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjSetActive(virStoragePoolObjPtr obj,
+   bool active);
+
+unsigned int
+virStoragePoolObjGetAsyncjobs(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjIncrAsyncjobs(virStoragePoolObjPtr obj);
+
+void
+virStoragePoolObjDecrAsyncjobs(virStoragePoolObjPtr obj);
 
 int
 virStoragePoolObjLoadAllConfigs(virStoragePoolObjListPtr pools,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4cca0ca..edd3174 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1009,10 +1009,17 @@ virSecretObjSetValueSize;
 # conf/virstorageobj.h
 virStoragePoolObjAssignDef;
 virStoragePoolObjClearVols;
+virStoragePoolObjDecrAsyncjobs;
 virStoragePoolObjDeleteDef;
 virStoragePoolObjFindByName;
 virStoragePoolObjFindByUUID;
+virStoragePoolObjGetAsyncjobs;
+virStoragePoolObjGetConfigFile;
+virStoragePoolObjGetDef;
 virStoragePoolObjGetNames;
+virStoragePoolObjGetNewDef;
+virStoragePoolObjIncrAsyncjobs;
+virStoragePoolObjIsActive;
 virStoragePoolObjIsDuplicate;
 virStoragePoolObjListExport;
 virStoragePoolObjListFree;
@@ -1023,7 +1030,10 @@ virStoragePoolObjNumOfStoragePools;
 virStoragePoolObjNumOfVolumes;
 virStoragePoolObjRemove;
 virStoragePoolObjSaveDef;
+virStoragePoolObjSetActive;
+virStoragePoolObjSetConfigFile;
 virStoragePoolObjSourceFindDuplicate;
+virStoragePoolObjStealNewDef;
 virStoragePoolObjUnlock;
 virStoragePoolObjVolumeGetNames;
 virStoragePoolObjVolumeListExport;
-- 
2.9.3

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

[libvirt] [PATCH 06/19] storage: Fix return value checks for virAsprintf

[John Ferlan]

Use the < 0 rather than == -1 (consistently) for virAsprintf errors.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_logical.c  | 2 +-
 src/storage/storage_backend_rbd.c  | 8 
 src/storage/storage_backend_sheepdog.c | 4 ++--
 src/storage/storage_backend_zfs.c  | 4 ++--
 src/storage/storage_util.c | 6 +++---
 src/test/test_driver.c | 9 +++--
 6 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index ed26c24..67f70e5 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -948,7 +948,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
 VIR_FREE(vol->target.path);
 if (virAsprintf(>target.path, "%s/%s",
 pool->def->target.path,
-vol->name) == -1)
+vol->name) < 0)
 return -1;
 
 cmd = virCommandNewArgList(LVCREATE,
diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index c806d6d..7b8887b 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -404,13 +404,13 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr 
vol,
 VIR_FREE(vol->target.path);
 if (virAsprintf(>target.path, "%s/%s",
 pool->def->source.name,
-vol->name) == -1)
+vol->name) < 0)
 goto cleanup;
 
 VIR_FREE(vol->key);
 if (virAsprintf(>key, "%s/%s",
 pool->def->source.name,
-vol->name) == -1)
+vol->name) < 0)
 goto cleanup;
 
 ret = 0;
@@ -662,13 +662,13 @@ virStorageBackendRBDCreateVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 VIR_FREE(vol->target.path);
 if (virAsprintf(>target.path, "%s/%s",
 pool->def->source.name,
-vol->name) == -1)
+vol->name) < 0)
 return -1;
 
 VIR_FREE(vol->key);
 if (virAsprintf(>key, "%s/%s",
 pool->def->source.name,
-vol->name) == -1)
+vol->name) < 0)
 return -1;
 
 return 0;
diff --git a/src/storage/storage_backend_sheepdog.c 
b/src/storage/storage_backend_sheepdog.c
index a9a2301..b55d96a 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -249,7 +249,7 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 VIR_FREE(vol->key);
 if (virAsprintf(>key, "%s/%s",
-pool->def->source.name, vol->name) == -1)
+pool->def->source.name, vol->name) < 0)
 return -1;
 
 VIR_FREE(vol->target.path);
@@ -374,7 +374,7 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 VIR_FREE(vol->key);
 if (virAsprintf(>key, "%s/%s",
-pool->def->source.name, vol->name) == -1)
+pool->def->source.name, vol->name) < 0)
 goto cleanup;
 
 VIR_FREE(vol->target.path);
diff --git a/src/storage/storage_backend_zfs.c 
b/src/storage/storage_backend_zfs.c
index 004d95a..c6dae71 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -89,7 +89,7 @@ virStorageBackendZFSCheckPool(virStoragePoolObjPtr pool 
ATTRIBUTE_UNUSED,
 char *devpath;
 
 if (virAsprintf(, "/dev/zvol/%s",
-pool->def->source.name) == -1)
+pool->def->source.name) < 0)
 return -1;
 *isActive = virFileIsDir(devpath);
 VIR_FREE(devpath);
@@ -322,7 +322,7 @@ virStorageBackendZFSCreateVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 VIR_FREE(vol->target.path);
 if (virAsprintf(>target.path, "%s/%s",
-pool->def->target.path, vol->name) == -1)
+pool->def->target.path, vol->name) < 0)
 return -1;
 
 if (VIR_STRDUP(vol->key, vol->target.path) < 0)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 43f3561..08dca94 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1982,7 +1982,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
 while ((direrr = virDirRead(dh, , NULL)) > 0) {
 if (virAsprintf(, "%s/%s",
 pool->def->target.path,
-dent->d_name) == -1) {
+dent->d_name) < 0) {
 VIR_DIR_CLOSE(dh);
 return NULL;
 }
@@ -2082,7 +2082,7 @@ virStorageBackendVolCreateLocal(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 VIR_FREE(vol->target.path);
 if (virAsprintf(>target.path, "%s/%s",
 pool->def->target.path,
-vol->name) == -1)
+vol->name) < 0)
 return -1;
 
 if (virFileExists(vol->target.path)) {
@@ -3553,7 +3553,7 @@ 

[libvirt] [PATCH 08/19] storage: Use consistent variable names for driver

[John Ferlan]

A virStoragePoolObjPtr will be an 'obj'.

A virStoragePoolPtr will be a 'pool'.

A virStorageVolPtr will be a 'vol'.

A virStorageVolDefPtr will be a 'voldef'.

Signed-off-by: John Ferlan 
---
 src/storage/storage_driver.c | 1158 +-
 src/storage/storage_driver.h |4 +-
 2 files changed, 582 insertions(+), 580 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 2103ed1..6122396 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -87,45 +87,45 @@ static void storageDriverUnlock(void)
  * pools are removed.
  */
 static void
-virStoragePoolUpdateInactive(virStoragePoolObjPtr *poolptr)
+virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
 {
-virStoragePoolObjPtr pool = *poolptr;
+virStoragePoolObjPtr obj = *objptr;
 
-if (pool->configFile == NULL) {
-virStoragePoolObjRemove(>pools, pool);
-*poolptr = NULL;
-} else if (pool->newDef) {
-virStoragePoolDefFree(pool->def);
-pool->def = pool->newDef;
-pool->newDef = NULL;
+if (obj->configFile == NULL) {
+virStoragePoolObjRemove(>pools, obj);
+*objptr = NULL;
+} else if (obj->newDef) {
+virStoragePoolDefFree(obj->def);
+obj->def = obj->newDef;
+obj->newDef = NULL;
 }
 }
 
 
 static void
-storagePoolUpdateState(virStoragePoolObjPtr pool)
+storagePoolUpdateState(virStoragePoolObjPtr obj)
 {
 bool active = false;
 virStorageBackendPtr backend;
 char *stateFile;
 
 if (!(stateFile = virFileBuildPath(driver->stateDir,
-   pool->def->name, ".xml")))
+   obj->def->name, ".xml")))
 goto cleanup;
 
-if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
+if ((backend = virStorageBackendForType(obj->def->type)) == NULL) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Missing backend %d"), pool->def->type);
+   _("Missing backend %d"), obj->def->type);
 goto cleanup;
 }
 
 /* Backends which do not support 'checkPool' are considered
  * inactive by default. */
 if (backend->checkPool &&
-backend->checkPool(pool, ) < 0) {
+backend->checkPool(obj, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to initialize storage pool '%s': %s"),
-   pool->def->name, virGetLastErrorMessage());
+   obj->def->name, virGetLastErrorMessage());
 active = false;
 }
 
@@ -134,21 +134,21 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
  * continue with other pools.
  */
 if (active) {
-virStoragePoolObjClearVols(pool);
-if (backend->refreshPool(NULL, pool) < 0) {
+virStoragePoolObjClearVols(obj);
+if (backend->refreshPool(NULL, obj) < 0) {
 if (backend->stopPool)
-backend->stopPool(NULL, pool);
+backend->stopPool(NULL, obj);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to restart storage pool '%s': %s"),
-   pool->def->name, virGetLastErrorMessage());
+   obj->def->name, virGetLastErrorMessage());
 active = false;
 }
 }
 
-pool->active = active;
+obj->active = active;
 
-if (!pool->active)
-virStoragePoolUpdateInactive();
+if (!obj->active)
+virStoragePoolUpdateInactive();
 
  cleanup:
 if (!active && stateFile)
@@ -164,11 +164,11 @@ storagePoolUpdateAllState(void)
 size_t i;
 
 for (i = 0; i < driver->pools.count; i++) {
-virStoragePoolObjPtr pool = driver->pools.objs[i];
+virStoragePoolObjPtr obj = driver->pools.objs[i];
 
-virStoragePoolObjLock(pool);
-storagePoolUpdateState(pool);
-virStoragePoolObjUnlock(pool);
+virStoragePoolObjLock(obj);
+storagePoolUpdateState(obj);
+virStoragePoolObjUnlock(obj);
 }
 }
 
@@ -186,24 +186,24 @@ storageDriverAutostart(void)
 /* Ignoring NULL conn - let backends decide */
 
 for (i = 0; i < driver->pools.count; i++) {
-virStoragePoolObjPtr pool = driver->pools.objs[i];
+virStoragePoolObjPtr obj = driver->pools.objs[i];
 virStorageBackendPtr backend;
 bool started = false;
 
-virStoragePoolObjLock(pool);
-if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
-virStoragePoolObjUnlock(pool);
+virStoragePoolObjLock(obj);
+if ((backend = virStorageBackendForType(obj->def->type)) == NULL) {
+virStoragePoolObjUnlock(obj);
 continue;
 }
 
-if (pool->autostart &&
-!virStoragePoolObjIsActive(pool)) {
+if (obj->autostart &&
+

[libvirt] [PATCH 03/19] test: Cleanup exit/failure paths of some storage pool APIs

[John Ferlan]

Rework some of the test driver API's to remove the need to return failure
when testStoragePoolObjFindByName returns NULL rather than going to cleanup.
This removes the need for check for "if (obj)" and in some instances the
need to for a cleanup label and a local ret variable.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 49 -
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index c0e46af..efa54ff 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4060,14 +4060,12 @@ testStoragePoolLookupByUUID(virConnectPtr conn,
 virStoragePoolPtr ret = NULL;
 
 if (!(obj = testStoragePoolObjFindByUUID(privconn, uuid)))
-goto cleanup;
+return NULL;
 
 ret = virGetStoragePool(conn, obj->def->name, obj->def->uuid,
 NULL, NULL);
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
+virStoragePoolObjUnlock(obj);
 return ret;
 }
 
@@ -4081,14 +4079,12 @@ testStoragePoolLookupByName(virConnectPtr conn,
 virStoragePoolPtr ret = NULL;
 
 if (!(obj = testStoragePoolObjFindByName(privconn, name)))
-goto cleanup;
+return NULL;
 
 ret = virGetStoragePool(conn, obj->def->name, obj->def->uuid,
 NULL, NULL);
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
+virStoragePoolObjUnlock(obj);
 return ret;
 }
 
@@ -4210,13 +4206,11 @@ testStoragePoolIsPersistent(virStoragePoolPtr pool)
 int ret = -1;
 
 if (!(obj = testStoragePoolObjFindByUUID(privconn, pool->uuid)))
-goto cleanup;
+return -1;
 
 ret = obj->configFile ? 1 : 0;
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
+virStoragePoolObjUnlock(obj);
 return ret;
 }
 
@@ -4668,10 +4662,9 @@ testStoragePoolGetInfo(virStoragePoolPtr pool,
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
-int ret = -1;
 
 if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
+return -1;
 
 memset(info, 0, sizeof(virStoragePoolInfo));
 if (obj->active)
@@ -4681,12 +4674,9 @@ testStoragePoolGetInfo(virStoragePoolPtr pool,
 info->capacity = obj->def->capacity;
 info->allocation = obj->def->allocation;
 info->available = obj->def->available;
-ret = 0;
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
-return ret;
+virStoragePoolObjUnlock(obj);
+return 0;
 }
 
 
@@ -4701,13 +4691,11 @@ testStoragePoolGetXMLDesc(virStoragePoolPtr pool,
 virCheckFlags(0, NULL);
 
 if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
+return NULL;
 
 ret = virStoragePoolDefFormat(obj->def);
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
+virStoragePoolObjUnlock(obj);
 return ret;
 }
 
@@ -4718,22 +4706,18 @@ testStoragePoolGetAutostart(virStoragePoolPtr pool,
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
-int ret = -1;
 
 if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
+return -1;
 
 if (!obj->configFile) {
 *autostart = 0;
 } else {
 *autostart = obj->autostart;
 }
-ret = 0;
 
- cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
-return ret;
+virStoragePoolObjUnlock(obj);
+return 0;
 }
 
 
@@ -4746,7 +4730,7 @@ testStoragePoolSetAutostart(virStoragePoolPtr pool,
 int ret = -1;
 
 if (!(obj = testStoragePoolObjFindByName(privconn, pool->name)))
-goto cleanup;
+return -1;
 
 if (!obj->configFile) {
 virReportError(VIR_ERR_INVALID_ARG,
@@ -4759,8 +4743,7 @@ testStoragePoolSetAutostart(virStoragePoolPtr pool,
 ret = 0;
 
  cleanup:
-if (obj)
-virStoragePoolObjUnlock(obj);
+virStoragePoolObjUnlock(obj);
 return ret;
 }
 
-- 
2.9.3

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

[libvirt] [PATCH 05/19] test: Add testStorageVolDefFindByName for storage volume tests

[John Ferlan]

Remove repetitive code, replace with common function.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 56 +++---
 1 file changed, 21 insertions(+), 35 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 9918df6..460aa88 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4797,9 +4797,24 @@ testStoragePoolListAllVolumes(virStoragePoolPtr pool,
 }
 
 
+static virStorageVolDefPtr
+testStorageVolDefFindByName(virStoragePoolObjPtr obj,
+const char *name)
+{
+virStorageVolDefPtr privvol;
+
+if (!(privvol = virStorageVolDefFindByName(obj, name))) {
+virReportError(VIR_ERR_NO_STORAGE_VOL,
+   _("no storage vol with matching name '%s'"), name);
+}
+
+return privvol;
+}
+
+
 static virStorageVolPtr
 testStorageVolLookupByName(virStoragePoolPtr pool,
-   const char *name ATTRIBUTE_UNUSED)
+   const char *name)
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
@@ -4809,13 +4824,8 @@ testStorageVolLookupByName(virStoragePoolPtr pool,
 if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name)))
 return NULL;
 
-privvol = virStorageVolDefFindByName(obj, name);
-
-if (!privvol) {
-virReportError(VIR_ERR_NO_STORAGE_VOL,
-   _("no storage vol with matching name '%s'"), name);
+if (!(privvol = testStorageVolDefFindByName(obj, name)))
 goto cleanup;
-}
 
 ret = virGetStorageVol(pool->conn, obj->def->name,
privvol->name, privvol->key,
@@ -5044,14 +5054,8 @@ testStorageVolDelete(virStorageVolPtr vol,
 if (!(obj = testStoragePoolObjFindActiveByName(privconn, vol->pool)))
 return -1;
 
-privvol = virStorageVolDefFindByName(obj, vol->name);
-
-if (privvol == NULL) {
-virReportError(VIR_ERR_NO_STORAGE_VOL,
-   _("no storage vol with matching name '%s'"),
-   vol->name);
+if (!(privvol = testStorageVolDefFindByName(obj, vol->name)))
 goto cleanup;
-}
 
 obj->def->allocation -= privvol->target.allocation;
 obj->def->available = (obj->def->capacity - obj->def->allocation);
@@ -5099,14 +5103,8 @@ testStorageVolGetInfo(virStorageVolPtr vol,
 if (!(obj = testStoragePoolObjFindActiveByName(privconn, vol->pool)))
 return -1;
 
-privvol = virStorageVolDefFindByName(obj, vol->name);
-
-if (privvol == NULL) {
-virReportError(VIR_ERR_NO_STORAGE_VOL,
-   _("no storage vol with matching name '%s'"),
-   vol->name);
+if (!(privvol = testStorageVolDefFindByName(obj, vol->name)))
 goto cleanup;
-}
 
 memset(info, 0, sizeof(*info));
 info->type = testStorageVolumeTypeForPool(obj->def->type);
@@ -5134,14 +5132,8 @@ testStorageVolGetXMLDesc(virStorageVolPtr vol,
 if (!(obj = testStoragePoolObjFindActiveByName(privconn, vol->pool)))
 return NULL;
 
-privvol = virStorageVolDefFindByName(obj, vol->name);
-
-if (privvol == NULL) {
-virReportError(VIR_ERR_NO_STORAGE_VOL,
-   _("no storage vol with matching name '%s'"),
-   vol->name);
+if (!(privvol = testStorageVolDefFindByName(obj, vol->name)))
 goto cleanup;
-}
 
 ret = virStorageVolDefFormat(obj->def, privvol);
 
@@ -5162,14 +5154,8 @@ testStorageVolGetPath(virStorageVolPtr vol)
 if (!(obj = testStoragePoolObjFindActiveByName(privconn, vol->pool)))
 return NULL;
 
-privvol = virStorageVolDefFindByName(obj, vol->name);
-
-if (privvol == NULL) {
-virReportError(VIR_ERR_NO_STORAGE_VOL,
-   _("no storage vol with matching name '%s'"),
-   vol->name);
+if (!(privvol = testStorageVolDefFindByName(obj, vol->name)))
 goto cleanup;
-}
 
 ignore_value(VIR_STRDUP(ret, privvol->target.path));
 
-- 
2.9.3

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

[libvirt] [PATCH 14/19] storage: Introduce storage volume add, delete, count APIs

[John Ferlan]

Create/use virStoragePoolObjAddVol in order to add volumes onto list.

Create/use virStoragePoolObjRemoveVol in order to remove volumes from list.

Create/use virStoragePoolObjGetVolumesCount to get count of volumes on list.

For the storage driver, the logic alters when the volumes.obj list grows
to after we've fetched the volobj. This is an optimization of sorts, but
also doesn't "needlessly" grow the volumes.objs list and then just decr
the count if the virGetStorageVol fails.

Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c   | 37 
 src/conf/virstorageobj.h   | 11 +++
 src/libvirt_private.syms   |  3 ++
 src/storage/storage_backend_disk.c |  5 ++--
 src/storage/storage_backend_gluster.c  |  3 +-
 src/storage/storage_backend_logical.c  |  3 +-
 src/storage/storage_backend_mpath.c|  3 +-
 src/storage/storage_backend_rbd.c  |  4 +--
 src/storage/storage_backend_sheepdog.c |  4 +--
 src/storage/storage_backend_zfs.c  |  5 +---
 src/storage/storage_driver.c   | 53 +-
 src/storage/storage_util.c |  4 +--
 src/test/test_driver.c | 18 
 13 files changed, 81 insertions(+), 72 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 69ed66d..cc3464e 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -305,6 +305,43 @@ virStoragePoolObjClearVols(virStoragePoolObjPtr obj)
 }
 
 
+int
+virStoragePoolObjAddVol(virStoragePoolObjPtr obj,
+virStorageVolDefPtr voldef)
+{
+if (VIR_APPEND_ELEMENT(obj->volumes.objs, obj->volumes.count, voldef) < 0)
+return -1;
+return 0;
+}
+
+
+void
+virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj,
+   virStorageVolDefPtr voldef)
+{
+virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
+size_t i;
+
+for (i = 0; i < obj->volumes.count; i++) {
+if (obj->volumes.objs[i] == voldef) {
+VIR_INFO("Deleting volume '%s' from storage pool '%s'",
+ voldef->name, def->name);
+virStorageVolDefFree(voldef);
+
+VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count);
+return;
+}
+}
+}
+
+
+size_t
+virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj)
+{
+return obj->volumes.count;
+}
+
+
 virStorageVolDefPtr
 virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
   const char *key)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 3b6e395..d27ff57 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -127,6 +127,17 @@ virStoragePoolObjPtr
 virStoragePoolObjFindByName(virStoragePoolObjListPtr pools,
 const char *name);
 
+int
+virStoragePoolObjAddVol(virStoragePoolObjPtr obj,
+virStorageVolDefPtr voldef);
+
+void
+virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj,
+   virStorageVolDefPtr voldef);
+
+size_t
+virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj);
+
 virStorageVolDefPtr
 virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
   const char *key);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e8b4eda..03777a3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1007,6 +1007,7 @@ virSecretObjSetValueSize;
 
 
 # conf/virstorageobj.h
+virStoragePoolObjAddVol;
 virStoragePoolObjAssignDef;
 virStoragePoolObjClearVols;
 virStoragePoolObjDecrAsyncjobs;
@@ -1019,6 +1020,7 @@ virStoragePoolObjGetConfigFile;
 virStoragePoolObjGetDef;
 virStoragePoolObjGetNames;
 virStoragePoolObjGetNewDef;
+virStoragePoolObjGetVolumesCount;
 virStoragePoolObjIncrAsyncjobs;
 virStoragePoolObjIsActive;
 virStoragePoolObjIsDuplicate;
@@ -1030,6 +1032,7 @@ virStoragePoolObjLock;
 virStoragePoolObjNumOfStoragePools;
 virStoragePoolObjNumOfVolumes;
 virStoragePoolObjRemove;
+virStoragePoolObjRemoveVol;
 virStoragePoolObjSaveDef;
 virStoragePoolObjSetActive;
 virStoragePoolObjSetAutostart;
diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index e8f67bb..0bf5567 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -65,8 +65,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
 if (VIR_ALLOC(vol) < 0)
 return -1;
 if (VIR_STRDUP(vol->name, partname) < 0 ||
-VIR_APPEND_ELEMENT_COPY(pool->volumes.objs,
-pool->volumes.count, vol) < 0) {
+virStoragePoolObjAddVol(pool, vol) < 0) {
 virStorageVolDefFree(vol);
 return -1;
 }
@@ -595,7 +594,7 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool,
 break;
 }
 }
-if (i == 

[libvirt] [PATCH 02/19] test: Use consistent variable names for storage test driver APIs

[John Ferlan]

A virStoragePoolObjPtr will be an 'obj'.

A virStoragePoolPtr will be a 'pool'.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 443 -
 1 file changed, 219 insertions(+), 224 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 548f318..c0e46af 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -565,7 +565,7 @@ static const char *defaultPoolSourcesNetFSXML =
 static const unsigned long long defaultPoolCap = (100 * 1024 * 1024 * 1024ull);
 static const unsigned long long defaultPoolAlloc;
 
-static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr pool);
+static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr obj);
 static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);
 
 static virDomainObjPtr
@@ -1039,8 +1039,8 @@ testParseInterfaces(testDriverPtr privconn,
 static int
 testOpenVolumesForPool(const char *file,
xmlXPathContextPtr ctxt,
-   virStoragePoolObjPtr pool,
-   int poolidx)
+   virStoragePoolObjPtr obj,
+   int objidx)
 {
 char *vol_xpath;
 size_t i;
@@ -1049,7 +1049,7 @@ testOpenVolumesForPool(const char *file,
 virStorageVolDefPtr def = NULL;
 
 /* Find storage volumes */
-if (virAsprintf(_xpath, "/node/pool[%d]/volume", poolidx) < 0)
+if (virAsprintf(_xpath, "/node/pool[%d]/volume", objidx) < 0)
 goto error;
 
 num = virXPathNodeSet(vol_xpath, ctxt, );
@@ -1063,25 +1063,24 @@ testOpenVolumesForPool(const char *file,
 if (!node)
 goto error;
 
-def = virStorageVolDefParseNode(pool->def, ctxt->doc, node, 0);
+def = virStorageVolDefParseNode(obj->def, ctxt->doc, node, 0);
 if (!def)
 goto error;
 
 if (def->target.path == NULL) {
 if (virAsprintf(>target.path, "%s/%s",
-pool->def->target.path,
+obj->def->target.path,
 def->name) == -1)
 goto error;
 }
 
 if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0)
 goto error;
-if (VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, pool->volumes.count, 
def) < 0)
+if (VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, obj->volumes.count, 
def) < 0)
 goto error;
 
-pool->def->allocation += def->target.allocation;
-pool->def->available = (pool->def->capacity -
-pool->def->allocation);
+obj->def->allocation += def->target.allocation;
+obj->def->available = (obj->def->capacity - obj->def->allocation);
 def = NULL;
 }
 
@@ -4000,14 +3999,14 @@ testInterfaceDestroy(virInterfacePtr iface,
  */
 
 static int
-testStoragePoolObjSetDefaults(virStoragePoolObjPtr pool)
+testStoragePoolObjSetDefaults(virStoragePoolObjPtr obj)
 {
 
-pool->def->capacity = defaultPoolCap;
-pool->def->allocation = defaultPoolAlloc;
-pool->def->available = defaultPoolCap - defaultPoolAlloc;
+obj->def->capacity = defaultPoolCap;
+obj->def->allocation = defaultPoolAlloc;
+obj->def->available = defaultPoolCap - defaultPoolAlloc;
 
-return VIR_STRDUP(pool->configFile, "");
+return VIR_STRDUP(obj->configFile, "");
 }
 
 
@@ -4015,18 +4014,18 @@ static virStoragePoolObjPtr
 testStoragePoolObjFindByName(testDriverPtr privconn,
  const char *name)
 {
-virStoragePoolObjPtr pool;
+virStoragePoolObjPtr obj;
 
 testDriverLock(privconn);
-pool = virStoragePoolObjFindByName(>pools, name);
+obj = virStoragePoolObjFindByName(>pools, name);
 testDriverUnlock(privconn);
 
-if (!pool)
+if (!obj)
 virReportError(VIR_ERR_NO_STORAGE_POOL,
_("no storage pool with matching name '%s'"),
name);
 
-return pool;
+return obj;
 }
 
 
@@ -4034,21 +4033,21 @@ static virStoragePoolObjPtr
 testStoragePoolObjFindByUUID(testDriverPtr privconn,
  const unsigned char *uuid)
 {
-virStoragePoolObjPtr pool;
+virStoragePoolObjPtr obj;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
 testDriverLock(privconn);
-pool = virStoragePoolObjFindByUUID(>pools, uuid);
+obj = virStoragePoolObjFindByUUID(>pools, uuid);
 testDriverUnlock(privconn);
 
-if (!pool) {
+if (!obj) {
 virUUIDFormat(uuid, uuidstr);
 virReportError(VIR_ERR_NO_STORAGE_POOL,
_("no storage pool with matching uuid '%s'"),
uuidstr);
 }
 
-return pool;
+return obj;
 }
 
 
@@ -4057,18 +4056,18 @@ testStoragePoolLookupByUUID(virConnectPtr conn,
 const unsigned char *uuid)
 {
 testDriverPtr privconn = conn->privateData;
-virStoragePoolObjPtr pool;
+virStoragePoolObjPtr obj;
 

[libvirt] [PATCH 00/19] Start altering storage code to privatize of the object

[John Ferlan]

Begin the process of making adjustments to the storage pool and volume code
in order to privatize the virStoragePoolObj and virStoragePoolObjList

Didn't want to post 40+ patches at one time...  This is the first storage
pile including some test driver cleanups w/r/t storage code.

John Ferlan (19):
  test: Fix up formatting in storage test API's
  test: Use consistent variable names for storage test driver APIs
  test: Cleanup exit/failure paths of some storage pool APIs
  test: Add helpers to fetch active/inactive storage pool by name
  test: Add testStorageVolDefFindByName for storage volume tests
  storage: Fix return value checks for virAsprintf
  storage: Use consistent variable names in virstorageobj
  storage: Use consistent variable names for driver
  storage: Alter volume num, name, and export API's to just take obj
  storage: Create accessor API's for virStoragePoolObj
  storage: Introduce virStoragePoolObjNew
  storage: Introduce virStoragePoolObj{Get|Set}Autostart
  storage: Move autostartLink deletion to virstorageobj
  storage: Introduce storage volume add, delete, count APIs
  storage: Introduce virStoragePoolObjForEachVolume
  storage: Use virStoragePoolObj accessors for driver
  storage: Use virStoragePoolObj accessors for storage test API's
  storage: Use virStoragePoolObj accessors for storage_util
  storage: Change storage_util to use obj instead of pool

 src/conf/virstorageobj.c   |  568 +-
 src/conf/virstorageobj.h   |   84 +-
 src/libvirt_private.syms   |   16 +
 src/storage/storage_backend_disk.c |   31 +-
 src/storage/storage_backend_gluster.c  |3 +-
 src/storage/storage_backend_logical.c  |5 +-
 src/storage/storage_backend_mpath.c|3 +-
 src/storage/storage_backend_rbd.c  |   12 +-
 src/storage/storage_backend_sheepdog.c |8 +-
 src/storage/storage_backend_zfs.c  |9 +-
 src/storage/storage_driver.c   | 1308 
 src/storage/storage_driver.h   |4 +-
 src/storage/storage_util.c |  200 ++---
 src/storage/storage_util.h |   30 +-
 src/test/test_driver.c |  780 +--
 15 files changed, 1614 insertions(+), 1447 deletions(-)

-- 
2.9.3

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

[libvirt] [PATCH 01/19] test: Fix up formatting in storage test API's

[John Ferlan]

Fix some spacing/formatting in the storage pool/vol test driver code.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 37 +++--
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2db3f7d..548f318 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1035,6 +1035,7 @@ testParseInterfaces(testDriverPtr privconn,
 return ret;
 }
 
+
 static int
 testOpenVolumesForPool(const char *file,
xmlXPathContextPtr ctxt,
@@ -1091,6 +1092,7 @@ testOpenVolumesForPool(const char *file,
 return ret;
 }
 
+
 static int
 testParseStorage(testDriverPtr privconn,
  const char *file,
@@ -1143,6 +1145,7 @@ testParseStorage(testDriverPtr privconn,
 return ret;
 }
 
+
 static int
 testParseNodedevs(testDriverPtr privconn,
   const char *file,
@@ -3996,8 +3999,8 @@ testInterfaceDestroy(virInterfacePtr iface,
  * Storage Driver routines
  */
 
-
-static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr pool)
+static int
+testStoragePoolObjSetDefaults(virStoragePoolObjPtr pool)
 {
 
 pool->def->capacity = defaultPoolCap;
@@ -4069,6 +4072,7 @@ testStoragePoolLookupByUUID(virConnectPtr conn,
 return ret;
 }
 
+
 static virStoragePoolPtr
 testStoragePoolLookupByName(virConnectPtr conn,
 const char *name)
@@ -4089,6 +4093,7 @@ testStoragePoolLookupByName(virConnectPtr conn,
 return ret;
 }
 
+
 static virStoragePoolPtr
 testStoragePoolLookupByVolume(virStorageVolPtr vol)
 {
@@ -4159,6 +4164,7 @@ testConnectListDefinedStoragePools(virConnectPtr conn,
 return n;
 }
 
+
 static int
 testConnectListAllStoragePools(virConnectPtr conn,
virStoragePoolPtr **pools,
@@ -4177,7 +4183,9 @@ testConnectListAllStoragePools(virConnectPtr conn,
 return ret;
 }
 
-static int testStoragePoolIsActive(virStoragePoolPtr pool)
+
+static int
+testStoragePoolIsActive(virStoragePoolPtr pool)
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
@@ -4194,7 +4202,9 @@ static int testStoragePoolIsActive(virStoragePoolPtr pool)
 return ret;
 }
 
-static int testStoragePoolIsPersistent(virStoragePoolPtr pool)
+
+static int
+testStoragePoolIsPersistent(virStoragePoolPtr pool)
 {
 testDriverPtr privconn = pool->conn->privateData;
 virStoragePoolObjPtr obj;
@@ -4212,7 +4222,6 @@ static int testStoragePoolIsPersistent(virStoragePoolPtr 
pool)
 }
 
 
-
 static int
 testStoragePoolCreate(virStoragePoolPtr pool,
   unsigned int flags)
@@ -4247,6 +4256,7 @@ testStoragePoolCreate(virStoragePoolPtr pool,
 return ret;
 }
 
+
 static char *
 testConnectFindStoragePoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
   const char *type,
@@ -4401,6 +4411,7 @@ testStoragePoolCreateXML(virConnectPtr conn,
 return ret;
 }
 
+
 static virStoragePoolPtr
 testStoragePoolDefineXML(virConnectPtr conn,
  const char *xml,
@@ -4448,6 +4459,7 @@ testStoragePoolDefineXML(virConnectPtr conn,
 return ret;
 }
 
+
 static int
 testStoragePoolUndefine(virStoragePoolPtr pool)
 {
@@ -4480,6 +4492,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
 return ret;
 }
 
+
 static int
 testStoragePoolBuild(virStoragePoolPtr pool,
  unsigned int flags)
@@ -4677,6 +4690,7 @@ testStoragePoolGetInfo(virStoragePoolPtr pool,
 return ret;
 }
 
+
 static char *
 testStoragePoolGetXMLDesc(virStoragePoolPtr pool,
   unsigned int flags)
@@ -4698,6 +4712,7 @@ testStoragePoolGetXMLDesc(virStoragePoolPtr pool,
 return ret;
 }
 
+
 static int
 testStoragePoolGetAutostart(virStoragePoolPtr pool,
 int *autostart)
@@ -4722,6 +4737,7 @@ testStoragePoolGetAutostart(virStoragePoolPtr pool,
 return ret;
 }
 
+
 static int
 testStoragePoolSetAutostart(virStoragePoolPtr pool,
 int autostart)
@@ -4832,6 +4848,7 @@ testStoragePoolListAllVolumes(virStoragePoolPtr obj,
 return ret;
 }
 
+
 static virStorageVolPtr
 testStorageVolLookupByName(virStoragePoolPtr pool,
const char *name ATTRIBUTE_UNUSED)
@@ -4905,6 +4922,7 @@ testStorageVolLookupByKey(virConnectPtr conn,
 return ret;
 }
 
+
 static virStorageVolPtr
 testStorageVolLookupByPath(virConnectPtr conn,
const char *path)
@@ -4941,6 +4959,7 @@ testStorageVolLookupByPath(virConnectPtr conn,
 return ret;
 }
 
+
 static virStorageVolPtr
 testStorageVolCreateXML(virStoragePoolPtr pool,
 const char *xmldesc,
@@ -5007,6 +5026,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool,
 return ret;
 }
 
+
 static virStorageVolPtr
 testStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 const char *xmldesc,
@@ -5084,6 +5104,7 @@ 

Re: [libvirt] [PATCH] Add "io" option to virsh attach-disk sub-command.

[Daniel P. Berrange]

On Tue, May 09, 2017 at 07:51:00AM -0700, Gordon Messmer wrote:
> On 05/09/2017 07:38 AM, Daniel P. Berrange wrote:
> > I think you can use the --iothread flag to "virsh attach-disk" to set
> > this parameter. virsh doesn't set it if not specified, since it tries
> > to avoid making policy decisions, like virt-manager/virt-install do
> 
> 
> That option sets the iothread attribute, which "attribute assigns the disk
> to an IOThread as defined by the range for the domain iothreads."  That's
> not the same as switching between native AIO and threads IO.

Opps, yes, mixed terminology. You're right - virsh is lacking this
feature. Feel free to send a patch to add a '--io' flag to the
attach-disk command if you want to try fixing this.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] Add "io" option to virsh attach-disk sub-command.

[Gordon Messmer]

On 05/09/2017 07:38 AM, Daniel P. Berrange wrote:

I think you can use the --iothread flag to "virsh attach-disk" to set
this parameter. virsh doesn't set it if not specified, since it tries
to avoid making policy decisions, like virt-manager/virt-install do



That option sets the iothread attribute, which "attribute assigns the 
disk to an IOThread as defined by the range for the domain iothreads."  
That's not the same as switching between native AIO and threads IO.


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

Re: [libvirt] [PATCH] Add "io" option to virsh attach-disk sub-command.

[Daniel P. Berrange]

On Tue, May 09, 2017 at 07:31:30AM -0700, Gordon Messmer wrote:
> (Apparently I managed to screw up sending that, twice.   Long weekend.)

No problem. Your first posting got blocked into the moderator queue
since you weren't a subscriber. One of the list admins then approved
it, not realizing you had already subscribed to the list & resent the
patch

> Please consider adding the option of setting the "io" attribute when
> attaching disks to guests.  Under Red Hat (unsure if this is true under
> vanilla libvirt), virt-install and virt-manager both default to explicitly
> setting "io='native'" in the disk "driver" tag. virsh, however, does not and
> also does not provide an option to specify that setting at all.  As a
> result, disks use a different IO mechanism (the default, "threads") when
> attached post-setup using virsh.  Adding this option allows users to keep
> disk performance consistent for disks attached at install, and those
> attached afterward.

I think you can use the --iothread flag to "virsh attach-disk" to set
this parameter. virsh doesn't set it if not specified, since it tries
to avoid making policy decisions, like virt-manager/virt-install do

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] Add "io" option to virsh attach-disk sub-command.

[Gordon Messmer]

(Apparently I managed to screw up sending that, twice.   Long weekend.)

Please consider adding the option of setting the "io" attribute when 
attaching disks to guests.  Under Red Hat (unsure if this is true under 
vanilla libvirt), virt-install and virt-manager both default to 
explicitly setting "io='native'" in the disk "driver" tag. virsh, 
however, does not and also does not provide an option to specify that 
setting at all.  As a result, disks use a different IO mechanism (the 
default, "threads") when attached post-setup using virsh.  Adding this 
option allows users to keep disk performance consistent for disks 
attached at install, and those attached afterward.


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

Re: [libvirt] [PATCH] Adding POWER9 cpu model to cpu_map.xml

[Andrea Bolognani]

On Tue, 2017-05-09 at 05:04 -0400, Kothapally Madhu Pavan wrote:
> As POWER9 model is not available in cpu_map.xml virsh capabilities
> donot display the cpu model and vendor details. This patch
> provides those details
> ---
>  src/cpu/cpu_map.xml |5 +
>  1 file changed, 5 insertions(+)

Acked-by: Andrea Bolognani 


I've pushed this, thanks for your contribution :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v3 6/6] docs: Document the mediated devices within the nodedev driver

[Pavel Hrdina]

On Wed, Apr 26, 2017 at 04:55:33PM +0200, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  docs/drvnodedev.html.in | 164 
> +++-
>  1 file changed, 163 insertions(+), 1 deletion(-)

s/3.3.0/3.4.0/ for the whole patch.

Please add the "mdev" capability into the list of valid capability
types for virsh nodedev-list command.

> 
> diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in
> index 0a3870343..8f820cca1 100644
> --- a/docs/drvnodedev.html.in
> +++ b/docs/drvnodedev.html.in
> @@ -9,7 +9,7 @@
>(historically also referred to as node devices) like USB, PCI, SCSI, 
> and
>network devices. This also includes various virtualization capabilities
>which the aforementioned devices provide for utilization, for example
> -  SR-IOV, NPIV, DRM, etc.
> +  SR-IOV, NPIV, MDEV, DRM, etc.
>  
>  
>  
> @@ -75,6 +75,7 @@
>storage (Since 1.0.4),
>scsi_generic (Since 1.0.7),
>drm (Since 3.1.0), and
> +  mdev (Since 3.3.0).
>This element can be nested in which case it further specifies a
>device's capability. Refer to specific device types to see more 
> values
>for the type attribute which are exclusive.
> @@ -185,5 +186,166 @@
>  ...
>  device
>  
> +MDEV capability
> +
> +  A PCI device capable of creating mediated devices will include a nested
> +  capability mdev_types which enumerates all supported mdev
> +  types on the physical device, along with the type attributes available
> +  through sysfs:
> +
> +
> +
> +  type
> +  
> +This element describes a mediated device type which acts as an
> +abstract template defining a resource allocation for instances of 
> this
> +device type. The element has one attribute id which 
> holds
> +an official vendor-supplied identifier for the type.
> +Since 3.3.0
> +  
> +
> +  name
> +  
> +The name element holds a vendor-supplied code name for
> +the given mediated device type. This is an optional element.
> +Since 3.3.0
> +  
> +
> +  deviceAPI
> +  
> +The value of this element describes how an instance of the given type
> +will be presented to the guest by the VFIO framework.
> +Since 3.3.0
> +  
> +
> +  availableInstances
> +  
> +This element reports the current state of resource allocation. In 
> other
> +words, how many instances of the given type can still be successfully
> +created on the physical device.
> +Since 3.3.0
> +  
> +
> +
> +
> +  For a more info about mediated devices, refer to the
> +  paragraph below.
> +
> +
> +
> +device
> +...
> +  driver
> +namenvidia/name
> +  /driver
> +  capability type='pci'
> +...
> +capability type='mdev_types'
> +  type id='nvidia-11'
> +nameGRID M60-0B/name
> +deviceAPIvfio-pci/deviceAPI
> +availableInstances16/availableInstances
> +  /type
> +  !-- Here would come the rest of the available mdev types --
> +/capability
> +...
> +  /capability
> +/device
> +
> +Mediated devices (MDEVs)
> +
> +  Mediated devices (Since 3.3.0) are software
> +  devices defining resource allocation on the backing physical device 
> which
> +  in turn allows the parent physical device's resources to be divided 
> into
> +  several mediated devices, thus sharing the physical device's 
> performance
> +  among multiple guests. Unlike SR-IOV however, where a PCIe device 
> appears
> +  as multiple separate PCIe devices on the host's PCI bus, mediated 
> devices
> +  only appear on the mdev virtual bus. Therefore, no detach/reattach
> +  procedure from/to the host driver procedure is involved even though
> +  mediated devices are used in a direct device assignment manner.

Replace the  by .

> +
> +  The following sub-elements and attributes are exposed within the
> +  capability element:
> +
> +
> +
> +  type
> +  
> +This element describes a mediated device type which acts as an
> +abstract template defining a resource allocation for instances of 
> this
> +device type. The element has one attribute id which 
> holds
> +an official vendor-supplied identifier for the type.
> +Since 3.3.0
> +  
> +
> +  iommuGroup
> +  
> +This element supports a single attribute number which 
> holds
> +the IOMMU group number the mediated device belongs to.
> +Since 3.3.0
> +  
> +
> +
> +Example of a mediated device
> +
> +device
> +  namemdev_4b20d080_1b54_4048_85b3_a6a62d165c01/name
> +  
> path/sys/devices/pci:00/:00:02.0/4b20d080-1b54-4048-85b3-a6a62d165c01/path
> +  parentpci__06_00_0/parent
> +  driver
> +

Re: [libvirt] [PATCH v3 4/6] nodedev: Introduce the mdev capability to a PCI parent device

[Pavel Hrdina]

On Wed, Apr 26, 2017 at 04:55:31PM +0200, Erik Skultety wrote:
> The parent device needs to report the generic stuff about the supported
> mediated devices types, like device API, available instances, type name,
> etc. Therefore this patch introduces a new nested capability element of
> type 'mdev_types' with the resulting XML of the following format:
> 
> 
> ...
>   
>   ...
> 
>   
> optional_vendor_supplied_codename
> vfio-pci
> NUM
>   
>   ...
>   
>   ...

It's a commit message so it doesn't matter but I would indent the
place-holder dots :).

>   
> 
>   
> ...
> 
> 
> Signed-off-by: Erik Skultety 
> ---
>  docs/schemas/nodedev.rng   |  26 +
>  src/conf/node_device_conf.c| 104 ++
>  src/conf/node_device_conf.h|  15 +++
>  src/conf/virnodedeviceobj.c|   7 ++
>  src/libvirt_private.syms   |   1 +
>  src/node_device/node_device_udev.c | 119 
> +
>  .../pci__02_10_7_mdev_types.xml|  32 ++
>  tests/nodedevxml2xmltest.c |   1 +
>  8 files changed, 305 insertions(+)
>  create mode 100644 tests/nodedevschemadata/pci__02_10_7_mdev_types.xml
> 
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 0f90a73c8..e0a2c5032 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -205,6 +205,32 @@
>  
>  
>  
> +  
> +
> +  mdev_types
> +
> +
> +  
> +
> +  
> +
> +
> +  
> +
> +
> +  
> +vfio-pci
> +  
> +
> +
> +  
> +
> +  
> +
> +  
> +   
> +
> +
>
>  
>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 24cb6d66f..b3012821a 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -88,6 +88,26 @@ virNodeDevCapsDefParseString(const char *xpath,
>  }
>  
>  
> +static void
> +virNodeDevCapMdevTypeClear(virNodeDevCapMdevTypePtr type)
> +{
> +VIR_FREE(type->id);
> +VIR_FREE(type->name);
> +VIR_FREE(type->device_api);
> +}

There is no need for the extra Clear function since it's static
and used only by the Free function.

> +
> +
> +void
> +virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type)
> +{
> +if (!type)
> +return;
> +
> +virNodeDevCapMdevTypeClear(type);
> +VIR_FREE(type);
> +}
> +
> +
>  void
>  virNodeDeviceDefFree(virNodeDeviceDefPtr def)
>  {
> @@ -265,6 +285,27 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
>  virBufferAsprintf(buf, "\n",
>virPCIHeaderTypeToString(data->pci_dev.hdrType));
>  }
> +if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
> +virBufferAddLit(buf, "\n");
> +virBufferAdjustIndent(buf, 2);
> +for (i = 0; i < data->pci_dev.nmdev_types; i++) {
> +virNodeDevCapMdevTypePtr type = data->pci_dev.mdev_types[i];
> +virBufferEscapeString(buf, "\n", type->id);
> +virBufferAdjustIndent(buf, 2);
> +if (type->name)
> +virBufferAsprintf(buf, "%s\n",
> +  type->name);
> +virBufferAsprintf(buf, "%s\n",
> +  type->device_api);

We should use virBufferEscapeString for  and  as well,
the data stored in these variables are loaded from sysfs.

> +virBufferAsprintf(buf,
> +  
> "%u\n",
> +  type->available_instances);
> +virBufferAdjustIndent(buf, -2);
> +virBufferAddLit(buf, "\n");
> +}
> +virBufferAdjustIndent(buf, -2);
> +virBufferAddLit(buf, "\n");
> +}
>  if (data->pci_dev.nIommuGroupDevices) {
>  virBufferAsprintf(buf, "\n",
>data->pci_dev.iommuGroupNumber);
> @@ -1365,6 +1406,63 @@ 
> virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr ctxt,
>  
>  
>  static int
> +virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr ctxt,
> +  virNodeDevCapPCIDevPtr pci_dev)
> +{
> +int ret = -1;
> +xmlNodePtr orignode = NULL;
> +xmlNodePtr *nodes = NULL;
> +int nmdev_types = virXPathNodeSet("./type", ctxt, );
> +virNodeDevCapMdevTypePtr type = NULL;
> +size_t i;
> +
> +orignode = ctxt->node;
> +for (i = 0; i < nmdev_types; i++) {
> +ctxt->node = nodes[i];
> +
> +if (VIR_ALLOC(type) < 0)
> +goto cleanup;
> +
> +if (!(type->id = virXPathString("string(./@id[1])", ctxt))) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +

[libvirt] [PATCH] tests: fix virfilewrapper

[Roman Bogorodskiy]

If __lxstat() and __xstat() functions are not available, build fails with:

  CC   virfilewrapper.o
virfilewrapper.c:180:5: error: no previous prototype for function '__lxstat' 
[-Werror,-Wmissing-prototypes]
int __lxstat(int ver, const char *path, struct stat *sb)
^
virfilewrapper.c:208:5: error: no previous prototype for function '__xstat' 
[-Werror,-Wmissing-prototypes]
int __xstat(int ver, const char *path, struct stat *sb)

Luckily, we already check presence of these functions in configure
using AC_CHECK_FUNCS, so just don't wrap these if they're not available.

Signed-off-by: Roman Bogorodskiy 
---
Pushed under build-breaker fix.

 tests/virfilewrapper.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c
index 75c32bfca..5ea70b34d 100644
--- a/tests/virfilewrapper.c
+++ b/tests/virfilewrapper.c
@@ -177,6 +177,7 @@ int access(const char *path, int mode)
 return ret;
 }
 
+#ifdef HAVE___LXSTAT
 int __lxstat(int ver, const char *path, struct stat *sb)
 {
 int ret = -1;
@@ -190,6 +191,7 @@ int __lxstat(int ver, const char *path, struct stat *sb)
 
 return ret;
 }
+#endif /* HAVE___LXSTAT */
 
 int lstat(const char *path, struct stat *sb)
 {
@@ -205,6 +207,7 @@ int lstat(const char *path, struct stat *sb)
 return ret;
 }
 
+#ifdef HAVE___XSTAT
 int __xstat(int ver, const char *path, struct stat *sb)
 {
 int ret = -1;
@@ -218,6 +221,7 @@ int __xstat(int ver, const char *path, struct stat *sb)
 
 return ret;
 }
+#endif /* HAVE___XSTAT */
 
 int stat(const char *path, struct stat *sb)
 {
-- 
2.12.1

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

Re: [libvirt] [PATCH v3 2/6] nodedev: conf: Split PCI sub-capability parsing to separate methods

[Pavel Hrdina]

On Wed, Apr 26, 2017 at 04:55:29PM +0200, Erik Skultety wrote:
> Since there's at least SRIOV and MDEV sub-capabilities to be parsed,
> let's make the code more readable by splitting it to several logical
> blocks.
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/conf/node_device_conf.c | 130 
> ++--
>  1 file changed, 77 insertions(+), 53 deletions(-)
> 
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 85cfd8396..d70d9942c 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -1286,76 +1286,102 @@ virPCIEDeviceInfoParseXML(xmlXPathContextPtr ctxt,
>  
>  
>  static int
> -virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt,
> -xmlNodePtr node,
> -virNodeDevCapPCIDevPtr pci_dev)
> +virNodeDevPCICapSRIOVPhysicalParseXML(xmlXPathContextPtr ctxt,
> +  virNodeDevCapPCIDevPtr pci_dev)
>  {
> -char *maxFuncsStr = virXMLPropString(node, "maxCount");
> -char *type = virXMLPropString(node, "type");
> -xmlNodePtr *addresses = NULL;
> -xmlNodePtr orignode = ctxt->node;
> -int ret = -1;
> -size_t i = 0;
> -
> -ctxt->node = node;
> -
> -if (!type) {
> -virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing capability 
> type"));
> -goto out;
> -}
> -
> -if (STREQ(type, "phys_function")) {
> -xmlNodePtr address = virXPathNode("./address[1]", ctxt);
> +xmlNodePtr address = virXPathNode("./address[1]", ctxt);
>  
>  if (VIR_ALLOC(pci_dev->physical_function) < 0)
> -goto out;
> +return -1;
>  
>  if (!address) {
>  virReportError(VIR_ERR_XML_ERROR, "%s",
> _("Missing address in 'phys_function' 
> capability"));
> -goto out;
> +return -1;
>  }
>  
>  if (virPCIDeviceAddressParseXML(address,
>  pci_dev->physical_function) < 0)
> -goto out;
> +return -1;
>  
>  pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;

Wrong indentation of the function body.

Pavel


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

[libvirt] [PATCH] util: Define SYSFS_SYSTEM_PATH unconditionally in virhostcpu

[Martin Kletzander]

The code is already prepared to handle the non-existence of it.

Signed-off-by: Martin Kletzander 
---
Pushed under the build-breaker rule.

 src/util/virhostcpu.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 317c52410a0b..aa9cfeac203c 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -188,10 +188,15 @@ virHostCPUGetStatsFreeBSD(int cpuNum,

 #endif /* __FreeBSD__ */

+/*
+ * Even though it doesn't exist on some platforms, the code is adjusted for
+ * graceful handling of that so that we don't have too many stub functions.
+ */
+#define SYSFS_SYSTEM_PATH "/sys/devices/system"
+
 #ifdef __linux__
 # define CPUINFO_PATH "/proc/cpuinfo"
 # define PROCSTAT_PATH "/proc/stat"
-# define SYSFS_SYSTEM_PATH "/sys/devices/system"
 # define VIR_HOST_CPU_MASK_LEN 1024

 # define LINUX_NB_CPU_STATS 4
--
2.12.2

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

[libvirt] [PATCH] bhyve: add support for video device configuration

[Roman Bogorodskiy]

Connect domain XML  configuration (introduced in a
previous patch) to bhyve command generation.

Unfortunately, this option was documented just recently and at the time
of writing official online manpages didn't have it updated, so for now
one can refer to:

https://github.com/freebsd/freebsd/blob/master/usr.sbin/bhyve/bhyve.8#L327

for the detailed description of the possible vga configuration options.

Also, add some tests for this new feature.

Signed-off-by: Roman Bogorodskiy 
---
 src/bhyve/bhyve_command.c  |  4 ++
 .../bhyvexml2argv-vnc-vgaconf.args | 12 ++
 .../bhyvexml2argv-vnc-vgaconf.ldargs   |  1 +
 .../bhyvexml2argv-vnc-vgaconf.xml  | 31 
 tests/bhyvexml2argvtest.c  |  1 +
 .../bhyvexml2xmlout-vnc-vgaconf.xml| 43 ++
 tests/bhyvexml2xmltest.c   |  1 +
 7 files changed, 93 insertions(+)
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.xml
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf.xml

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index eae5cb3ca..f70e3bc60 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -408,6 +408,10 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def 
ATTRIBUTE_UNUSED,
_("Unsupported listen type"));
 }
 
+if (video->driver)
+virBufferAsprintf(, ",vga=%s",
+  
virDomainVideoVgaconfTypeToString(video->driver->vgaconf));
+
 virCommandAddArg(cmd, "-s");
 virCommandAddArgBuffer(cmd, );
 return 0;
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.args
new file mode 100644
index 0..70347ee0b
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.args
@@ -0,0 +1,12 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-u \
+-H \
+-P \
+-s 0:0,hostbridge \
+-l bootrom,/path/to/test.fd \
+-s 2:0,ahci,hd:/tmp/freebsd.img \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \
+-s 4:0,fbuf,tcp=127.0.0.1:5904,vga=off \
+-s 1,lpc bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.ldargs
new file mode 100644
index 0..421376db9
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.ldargs
@@ -0,0 +1 @@
+dummy
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.xml
new file mode 100644
index 0..f1bcd1bde
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf.xml
@@ -0,0 +1,31 @@
+
+  bhyve
+  df3be7e7-a104-11e3-aeb0-50e5492bd3dc
+  219136
+  1
+  
+hvm
+/path/to/test.fd
+  
+  
+
+  
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+
+
+  
+ 
+  
+
+  
+
diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index c8f8c685a..a369a447a 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -193,6 +193,7 @@ mymain(void)
 DO_TEST("net-e1000");
 DO_TEST("uefi");
 DO_TEST("vnc");
+DO_TEST("vnc-vgaconf");
 
 /* Address allocation tests */
 DO_TEST("addr-single-sata-disk");
diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf.xml 
b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf.xml
new file mode 100644
index 0..6cc1aa088
--- /dev/null
+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf.xml
@@ -0,0 +1,43 @@
+
+  bhyve
+  df3be7e7-a104-11e3-aeb0-50e5492bd3dc
+  219136
+  219136
+  1
+  
+hvm
+/path/to/test.fd
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+
+  
+  
+  
+  
+
+
+
+  
+
+
+  
+  
+  
+  
+
+
+  
+
+
+  
+
+  
+  
+
+  
+
diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
index b3759919e..70e4caeb3 100644
--- a/tests/bhyvexml2xmltest.c
+++ b/tests/bhyvexml2xmltest.c
@@ -105,6 +105,7 @@ mymain(void)
 DO_TEST_DIFFERENT("serial-grub");
 DO_TEST_DIFFERENT("serial-grub-nocons");
 DO_TEST_DIFFERENT("vnc");
+DO_TEST_DIFFERENT("vnc-vgaconf");
 
 /* Address allocation tests */
 DO_TEST_DIFFERENT("addr-single-sata-disk");
-- 
2.12.1

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

[libvirt] [PATCH 1/2] conf: add video driver configuration element

[Roman Bogorodskiy]

Add support for video driver configuration. In domain xml it looks like
this:

  

  

  

At present, the only supported configuration is 'vgaconf' that looks this way:



It was added with bhyve gop video in mind to allow users control how the
video device is exposed to the guest, specifically, how VGA I/O is
handled.

Signed-off-by: Roman Bogorodskiy 
---
 docs/schemas/domaincommon.rng | 13 +
 src/conf/domain_conf.c| 63 +--
 src/conf/domain_conf.h| 17 
 src/libvirt_private.syms  |  2 ++
 4 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 281309ec0..f45820383 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3278,6 +3278,19 @@
   
 
   
+  
+
+  
+
+  
+io
+on
+off
+  
+
+  
+
+  
 
   
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ff216e3a..2ab55b52f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -553,6 +553,11 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
   "virtio",
   "gop")
 
+VIR_ENUM_IMPL(virDomainVideoVgaconf, VIR_DOMAIN_VIDEO_VGACONF_LAST,
+  "io",
+  "on",
+  "off")
+
 VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST,
   "mouse",
   "tablet",
@@ -2280,6 +2285,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
 virDomainDeviceInfoClear(>info);
 
 VIR_FREE(def->accel);
+VIR_FREE(def->driver);
 VIR_FREE(def);
 }
 
@@ -13368,6 +13374,43 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 return def;
 }
 
+static virDomainVideoDriverDefPtr
+virDomainVideoDriverDefParseXML(xmlNodePtr node)
+{
+xmlNodePtr cur;
+virDomainVideoDriverDefPtr def;
+char *vgaconf = NULL;
+int val;
+
+cur = node->children;
+while (cur != NULL) {
+if (cur->type == XML_ELEMENT_NODE) {
+if (!vgaconf &&
+xmlStrEqual(cur->name, BAD_CAST "driver")) {
+vgaconf = virXMLPropString(cur, "vgaconf");
+}
+}
+cur = cur->next;
+}
+
+if (!vgaconf)
+return NULL;
+
+if (VIR_ALLOC(def) < 0)
+goto cleanup;
+
+if ((val = virDomainVideoVgaconfTypeFromString(vgaconf)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown vgaconf value '%s'"), vgaconf);
+goto cleanup;
+}
+def->vgaconf = val;
+
+ cleanup:
+VIR_FREE(vgaconf);
+return def;
+}
+
 static virDomainVideoDefPtr
 virDomainVideoDefParseXML(xmlNodePtr node,
   const virDomainDef *dom,
@@ -13405,6 +13448,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 }
 
 def->accel = virDomainVideoAccelDefParseXML(cur);
+def->driver = virDomainVideoDriverDefParseXML(cur);
 }
 }
 cur = cur->next;
@@ -22998,6 +23042,18 @@ virDomainVideoAccelDefFormat(virBufferPtr buf,
 virBufferAddLit(buf, "/>\n");
 }
 
+static void
+virDomainVideoDriverDefFormat(virBufferPtr buf,
+  virDomainVideoDriverDefPtr def)
+{
+virBufferAddLit(buf, "vgaconf) {
+virBufferAsprintf(buf, " vgaconf='%s'",
+  virDomainVideoVgaconfTypeToString(def->vgaconf));
+}
+virBufferAddLit(buf, "/>\n");
+}
+
 
 static int
 virDomainVideoDefFormat(virBufferPtr buf,
@@ -23028,10 +23084,13 @@ virDomainVideoDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, " heads='%u'", def->heads);
 if (def->primary)
 virBufferAddLit(buf, " primary='yes'");
-if (def->accel) {
+if (def->accel || def->driver) {
 virBufferAddLit(buf, ">\n");
 virBufferAdjustIndent(buf, 2);
-virDomainVideoAccelDefFormat(buf, def->accel);
+if (def->accel)
+virDomainVideoAccelDefFormat(buf, def->accel);
+if (def->driver)
+virDomainVideoDriverDefFormat(buf, def->driver);
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
 } else {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09fb7aada..cbf25a67b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1350,6 +1350,16 @@ typedef enum {
 } virDomainVideoType;
 
 
+typedef enum {
+VIR_DOMAIN_VIDEO_VGACONF_IO = 0,
+VIR_DOMAIN_VIDEO_VGACONF_ON,
+VIR_DOMAIN_VIDEO_VGACONF_OFF,
+
+VIR_DOMAIN_VIDEO_VGACONF_LAST
+} virDomainVideoVgaconf;
+
+VIR_ENUM_DECL(virDomainVideoVgaconf)
+
 typedef struct _virDomainVideoAccelDef virDomainVideoAccelDef;
 typedef 

Re: [libvirt] [PATCH] bhyve: add support for video device configuration

[Roman Bogorodskiy]

Sorry for the strange splitting of this (supposed-to-be) series,
apparently I have something weird happening on my system so
git-send-email authenticates only when sending the first patch :-(

Roman Bogorodskiy


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

[libvirt] [PATCH 1/3] bhyve: tests: add vnc test to bhyvexml2xmltest

[Roman Bogorodskiy]

Signed-off-by: Roman Bogorodskiy 
---
 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml | 41 +++
 tests/bhyvexml2xmltest.c  |  3 +-
 2 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml

diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml 
b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml
new file mode 100644
index 0..9e470e432
--- /dev/null
+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml
@@ -0,0 +1,41 @@
+
+  bhyve
+  df3be7e7-a104-11e3-aeb0-50e5492bd3dc
+  219136
+  219136
+  1
+  
+hvm
+/path/to/test.fd
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+
+  
+  
+  
+  
+
+
+
+  
+
+
+  
+  
+  
+  
+
+
+  
+
+
+  
+  
+
+  
+
diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
index d591576ca..b3759919e 100644
--- a/tests/bhyvexml2xmltest.c
+++ b/tests/bhyvexml2xmltest.c
@@ -104,6 +104,7 @@ mymain(void)
 DO_TEST_DIFFERENT("serial");
 DO_TEST_DIFFERENT("serial-grub");
 DO_TEST_DIFFERENT("serial-grub-nocons");
+DO_TEST_DIFFERENT("vnc");
 
 /* Address allocation tests */
 DO_TEST_DIFFERENT("addr-single-sata-disk");
@@ -127,7 +128,7 @@ mymain(void)
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
-VIR_TEST_MAIN(mymain)
+VIR_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/bhyvexml2argvmock.so")
 
 #else
 
-- 
2.12.1

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

Re: [libvirt] RFE: virsh list improvement (--description and --details)

[Andrea Bolognani]

On Mon, 2017-05-08 at 09:16 +0200, Przemysław Sztoch wrote:
> Dear Michal and Pavel,
> 
> We cover about 100 clients who have their own and simple CentOS KVM 
> installations.
> Their technical skills are far from writing python scripts. They expect 
> simple solutions.

They don't necessarily have to write the Python script
themselves, though :)

> Talking to our helpdesk, I found that 70% of libvirt and virtualization 
> problems are:
> A) lack of autostart activation on critical guests; then occasional 
> failures and reboots affect lack of automatic startup of key services,
> B) frequent overcommiting of allocated virtual processors and memory due 
> to the lack of basic planning and addition operations of local admin 
> stuff :-(,
> C) misconfiguration of qemu-agent, which affects many problems with safe 
> restart, snapshot, backup, etc. (the "Time" column is a perfect 
> diagnostic here)
> D) leaving unnecessary snapshots that lie unused after many months,
> E) live migration attempts that fail to put domain in a transient mode 
> leave the guests disappearing in unexplained circumstances after kvm 
> host restart :-)
> 
> Virtually all the above problems of everyday life, our helpdesk is now 
> able to diagnose by command:
> virsh list --details --managed-save
> By the way, they can easily update the documentation with one compact list.
> 
> I do not understand your dislike for the proposed changes. All the 
> members of our team and teams of our partners have been very 
> enthusiastic about the new functionality.
> You govern, so you have to decide. ;-)

The problem with your proposal is that it doesn't fit
neatly in a generic tool like virsh.

My suggestion would be to implement the script yourself,
then ship it to your clients and instruct them to run

  # virt-diagnostics.py

or whatever you end up calling it to obtain the
information you care about.

Doing so will allow you to have plenty of freedom when
it comes to tailoring the output for your specific needs
instead of having to keep the tool generic, which you
would have to do if you wanted it to be shipped with
libvirt, and it won't be any harder for your clients to
run it.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCHv4 1/6] conf: add to

[Andrea Bolognani]

On Wed, 2017-05-03 at 16:05 +0200, Ján Tomko wrote:
> Add a new  element with a driver attribute.
> 
> Possible values are qemu and kvm. With 'qemu', the I/O
> APIC can be put in the userspace even for KVM domains.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  docs/formatdomain.html.in  |  8 ++
>  docs/schemas/domaincommon.rng  | 15 ++
>  src/conf/domain_conf.c | 33 
>+-
>  src/conf/domain_conf.h | 11 
>  .../qemuxml2argv-intel-iommu-ioapic.xml| 29 +++
>  .../qemuxml2xmlout-intel-iommu-ioapic.xml  |  1 +
>  tests/qemuxml2xmltest.c|  1 +
>  7 files changed, 97 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-ioapic.xml
>  create mode 12 
>tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-ioapic.xml

Please make sure that only x86 guests can use the 
feature, and all other guests get an error instead.

Also I didn't check whether this is the case already, but
the feature should be advertised the same way  is,
and in particular it should show up in the capabilities XML.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH v2] Detect VMDK version 3 files

[Daniel P. Berrange]

The metadata libvirt cares about is identical for version 3
as for previous versions, so we merely need list the new
version number.

Signed-off-by: Daniel P. Berrange 
---
 src/util/virstoragefile.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 94a77ce..b43acf6 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -113,7 +113,7 @@ enum {
 BACKING_STORE_ERROR,
 };
 
-#define FILE_TYPE_VERSIONS_LAST 2
+#define FILE_TYPE_VERSIONS_LAST 3
 
 struct FileEncryptionInfo {
 int format; /* Encryption format to assign */
@@ -374,7 +374,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
 },
 [VIR_STORAGE_FILE_VMDK] = {
 0, "KDMV", NULL,
-LV_LITTLE_ENDIAN, 4, 4, {1, 2},
+LV_LITTLE_ENDIAN, 4, 4, {1, 2, 3},
 4+4+4, 8, 512, NULL, vmdk4GetBackingStore, NULL
 },
 };
-- 
2.9.3

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

Re: [libvirt] [PATCH] Don't use ceph-devel on Fedora

[Pavel Hrdina]

On Mon, May 08, 2017 at 10:56:47AM +0100, Daniel P. Berrange wrote:
> A previous commit changed the spec to use librbd1-devel on
> RHEL-7, since this replaces ceph-devel from RHEL-6:
> 
>   commit 6cfc8834c858849cc74c3082078dc91fb1cbae38
>   Author: Peter Krempa 
>   Date:   Thu Mar 5 11:40:54 2015 +0100
> 
> spec: Enable RBD storage driver in RHEL-7
> 
> Use correct package names too as they differ.
> 
> RHEL-7 inherited this split from Fedora though, so it should
> have also made Fedora use the new names.
> 
> This was missed, because Fedora still provides a (deprecated)
> back-compat RPM for ceph-devel that just pulls in librbd1-devel
> (and others).
> 
> Fixing this stops libvirt pulling Java into the build root in
> Fedora.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] How about fuzz testing on oss-fuzz?

[Daniel P. Berrange]

On Tue, May 09, 2017 at 11:12:24AM +0200, Michal Privoznik wrote:
> On 05/09/2017 11:01 AM, Daniel P. Berrange wrote:
> > On Fri, Mar 31, 2017 at 10:23:33AM +0200, Peter Krempa wrote:
> >> On Fri, Mar 31, 2017 at 03:57:41 -0400, Dan wrote:
> >>> Hi all,
> >>>
> >>> I have seen libxml2 has already been added as a project in oss-fuzz [1].
> >>> Any idea about libvirt? While we could do our own fuzzing of some form, do
> >>> we want to also try it out using google's free resource?
> >>
> >> The oss-fuzz project  requires you to integrate the project with
> >> the libfuzz fuzzer in the first place so you have to make it run locally
> >> first anyways.
> >>
> >> Doing it on the oss-fuzz project is still the step after that.
> > 
> > FYI, google is now offering rewards to projects that integrate
> > with oss-fuzz
> > 
> >   "To qualify for these rewards, a project needs to have a large
> >user base and/or be critical to global IT infrastructure. 
> >Eligible projects will receive $1,000 for initial integration,
> >and up to $20,000 for ideal integration (the final amount is
> >at our discretion). You have the option of donating these 
> >rewards to charity instead, and Google will double the amount."
> > 
> > I'd like to think libvirt qualifies under "large user base" and
> > "critical to global IT" given prevelance of the cloud these days,
> > but no guarantees
> > 
> >   
> > https://opensource.googleblog.com/2017/05/oss-fuzz-five-months-later-and.html
> 
> Right. I've read this on G+ during the weekend. And now that we have
> accepted a student for the fuzzing GSoC project, we can work towards
> that goal.
> 
> > 
> > Not that libvirt really has any current need for monetary funds. If it ever
> > came to pass, we could just have a poll amongst active contributors to
> > vote on suggestions of what todo with it (donate it, spend it, fund 
> > something,
> > etc).
> 
> I don't know any details, but I know from the past that receiving money
> for orgs wasn't trivial (at least for GSoC). We had to have an law
> entity that covers the project. Since there was none, we donated our
> mentor money to Tor foundation. But it has changed a while ago (again,
> at least for GSoC), so maybe we are eligible to receive money after all.

Yep, just telling Google to donate it directly to a charity of our
choosing would probably end up being the simplest option from a legal
pov, as it would avoid us handling it at all.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] How about fuzz testing on oss-fuzz?

[Michal Privoznik]

On 05/09/2017 11:01 AM, Daniel P. Berrange wrote:
> On Fri, Mar 31, 2017 at 10:23:33AM +0200, Peter Krempa wrote:
>> On Fri, Mar 31, 2017 at 03:57:41 -0400, Dan wrote:
>>> Hi all,
>>>
>>> I have seen libxml2 has already been added as a project in oss-fuzz [1].
>>> Any idea about libvirt? While we could do our own fuzzing of some form, do
>>> we want to also try it out using google's free resource?
>>
>> The oss-fuzz project  requires you to integrate the project with
>> the libfuzz fuzzer in the first place so you have to make it run locally
>> first anyways.
>>
>> Doing it on the oss-fuzz project is still the step after that.
> 
> FYI, google is now offering rewards to projects that integrate
> with oss-fuzz
> 
>   "To qualify for these rewards, a project needs to have a large
>user base and/or be critical to global IT infrastructure. 
>Eligible projects will receive $1,000 for initial integration,
>and up to $20,000 for ideal integration (the final amount is
>at our discretion). You have the option of donating these 
>rewards to charity instead, and Google will double the amount."
> 
> I'd like to think libvirt qualifies under "large user base" and
> "critical to global IT" given prevelance of the cloud these days,
> but no guarantees
> 
>   
> https://opensource.googleblog.com/2017/05/oss-fuzz-five-months-later-and.html

Right. I've read this on G+ during the weekend. And now that we have
accepted a student for the fuzzing GSoC project, we can work towards
that goal.

> 
> Not that libvirt really has any current need for monetary funds. If it ever
> came to pass, we could just have a poll amongst active contributors to
> vote on suggestions of what todo with it (donate it, spend it, fund something,
> etc).

I don't know any details, but I know from the past that receiving money
for orgs wasn't trivial (at least for GSoC). We had to have an law
entity that covers the project. Since there was none, we donated our
mentor money to Tor foundation. But it has changed a while ago (again,
at least for GSoC), so maybe we are eligible to receive money after all.

Michal

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

[libvirt] [PATCH] Adding POWER9 cpu model to cpu_map.xml

[Kothapally Madhu Pavan]

As POWER9 model is not available in cpu_map.xml virsh capabilities
donot display the cpu model and vendor details. This patch
provides those details
---
 src/cpu/cpu_map.xml |5 +
 1 file changed, 5 insertions(+)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 7d5540a..29b5b59 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -1571,6 +1571,11 @@
   
 
 
+
+  
+  
+
+
 
 
   

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

Re: [libvirt] How about fuzz testing on oss-fuzz?

[Daniel P. Berrange]

On Fri, Mar 31, 2017 at 10:23:33AM +0200, Peter Krempa wrote:
> On Fri, Mar 31, 2017 at 03:57:41 -0400, Dan wrote:
> > Hi all,
> > 
> > I have seen libxml2 has already been added as a project in oss-fuzz [1].
> > Any idea about libvirt? While we could do our own fuzzing of some form, do
> > we want to also try it out using google's free resource?
> 
> The oss-fuzz project  requires you to integrate the project with
> the libfuzz fuzzer in the first place so you have to make it run locally
> first anyways.
> 
> Doing it on the oss-fuzz project is still the step after that.

FYI, google is now offering rewards to projects that integrate
with oss-fuzz

  "To qualify for these rewards, a project needs to have a large
   user base and/or be critical to global IT infrastructure. 
   Eligible projects will receive $1,000 for initial integration,
   and up to $20,000 for ideal integration (the final amount is
   at our discretion). You have the option of donating these 
   rewards to charity instead, and Google will double the amount."

I'd like to think libvirt qualifies under "large user base" and
"critical to global IT" given prevelance of the cloud these days,
but no guarantees

  https://opensource.googleblog.com/2017/05/oss-fuzz-five-months-later-and.html

Not that libvirt really has any current need for monetary funds. If it ever
came to pass, we could just have a poll amongst active contributors to
vote on suggestions of what todo with it (donate it, spend it, fund something,
etc).

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] 答复: Re: [PATCH] qemu: change the name of tap device for a tapand bridge network

[Daniel P. Berrange]

FYI your replies to messages on the list are almost impossible
to read & understand.

Your email client is sending HTML and badly mangled plain text.
Please make it send plain text only to public mailing lists,
with line wrapping and sensibly trim & quote text you are
replying to.

I can't see any comment you made in this latest message so I can't
reply to you.

On Tue, May 09, 2017 at 08:44:55AM +0800, lu.zhip...@zte.com.cn wrote:
> On Mon, May 08, 2017 at 03:03:30PM +0800, ZhiPeng Lu wrote:＞＞ In 
> qemuProcessStop we explicitly remove the port from the openvswitch bridge 
> after＞＞ qemuProcessKill is called. But there is a certain interval of time 
> between＞＞ deleting tap device and removing it from bridge. The problem occurs 
> when two vms＞＞ start and shutdown with the same name's network interface 
> attached to the same＞＞ openvswitch bridge. When one vm with the nic named 
> "vnet0" stopping, it deleted＞＞tap device without timely removing the port 
> from bridge.＞＞.At this time, another vm created the tap device named "vnet0" 
> and added port to the＞＞ same bridge. Then, the first vm removed the port from 
> the same bridge.＞＞ Finally, the tap device of the second vm did not attached 
> to the bridge.＞＞ We need to delete the bridge port before deleting the tap 
> device instead of after.＞＞ So what's needed is to move the loop in 
> qemuProcessStop that cleans up＞＞ network interfaces so that it happens before 
> qemuProcessKill is called.＞This fix won't work correctly either. You cannot 
> assume that libvirt has＞control over when the QEMU process exits. It may exit 
> itself *before*＞libvirt runs any of its cleanup code.
> 
> 
> 
> 
> On Mon, May 08, 2017 at 06:18:25PM +0800, lu.zhip...@zte.com.cn wrote:＞ I may 
> not have described it clearly.  ＞ ＞ i need to ensure  the order of  adding 
> port to bridge and deleting from bridge.＞ ＞ i  rename tap device to avoid the 
> problem  in my first patch.i think it can solve the problem.＞ ＞ my second 
> patch may can't resolve the problem .＞ ＞ Do you  have any better ideas?
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
g> 
> 
> 
> 
> 
> 
> 为了让您的VPlat虚拟化故障得到高效的处理，请上报故障到: $VPlat技术支持。
> 
> 
> 芦志朋 luzhipeng
> 
> 
> 
> 
> 
> 
> IT开发工程师 IT Development
> Engineer
> 操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R＆D Institute/System Product
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 深圳市南山区科技南路55号中兴通讯研发大楼33楼 
> 33/F, R Building, ZTE
> Corporation Hi-tech Road South, 
> Hi-tech
> Industrial Park Nanshan District, Shenzhen, P.R.China, 518057 
> T: +86 755  F:+86 755  
> M: +86 xxx 
> E: lu.zhip...@zte.com.cn 
> www.zte.com.cn
> 
> 
> 
> 
> 
> 
> 原始邮件
> 
> 
> 
> 发件人： ＜berra...@redhat.com＞
> 收件人： ＜la...@laine.org＞
> 抄送人： ＜libvir-list@redhat.com＞芦志朋10108272
> 日 期 ：2017年05月02日 16:41
> 主 题 ：Re: [libvirt] [PATCH] qemu: change the name of tap device for a tapand 
> bridge network
> 
> 
> 
> 
> 
> On Fri, Apr 28, 2017 at 01:08:45PM -0400, Laine Stump wrote:
> ＞ On 04/28/2017 05:33 AM, Daniel P. Berrange wrote:
> ＞ ＞ On Fri, Apr 28, 2017 at 05:23:19PM +0800, ZhiPeng Lu wrote:
> ＞ ＞＞ Creating tap device and adding the device to bridge are not atomic 
> operation.
> ＞ ＞＞ Similarly deleting tap device and removing it from bridge are not atomic 
> operation.
> ＞ ＞＞ The Problem occurs when two vms start and shutdown. When one vm with the 
> nic
> ＞ ＞＞ named "vnet0" stopping, it deleted tap device but not removing port from 
> bridge.
> ＞ ＞＞ At this time, another vm created the tap device named "vnet0" and added 
> port to the
> ＞ ＞＞ same bridge. Then, the first vm deleted the tap device from the same 
> bridge.
> ＞ ＞＞ Finally, the tap device of the second vm don't attached to the bridge.
> ＞ ＞＞ So, we can add domid to vm's nic name. For example, the vm's domid is 1 
> and vnet0
> ＞ ＞＞ is renamed to vnet1.0.
> ＞ ＞ 
> ＞ ＞ Surely deleting the NIC automatically removes it from the bridge so we
> ＞ ＞ can just remove the code that delets the bridge port.
> ＞ 
> ＞ That is true when using a Linux host bridge, but I recall that for
> ＞ openvswitch (which I think is what ZhiPeng is using, based on an earlier
> ＞ patch), you must explicitly remove the port from the bridge - apparently
> ＞ the port is still there in openvswitch's table as some sort of "zombie"
> ＞ connection even after the tap device itself no longer exists.
> ＞ 
> ＞ 
> ＞ But instead of changing the naming scheme, maybe we should just delete
> ＞ the bridge port *before* deleting the tap device instead of after. (Am I
> ＞ recalling correctly that the tap device is deleted automatically when
> ＞ the qemu process is killed? If so, then what's needed is to move the
> ＞ loop in qemuProcessStop() that cleans up network interfaces so that it
> ＞ happens before qemuProcessKill() is called.
> 
> Agreed, we should just reverse the order.
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: 

Re: [libvirt] [PATCH v2 30/38] virNetClientStream: Wire up VIR_NET_STREAM_SKIP

[Michal Privoznik]

On 05/05/2017 05:43 PM, John Ferlan wrote:
> 
> 
> On 04/20/2017 06:01 AM, Michal Privoznik wrote:
>> Whenever server sends a client stream packet (either regular with
>> actual data or stream skip one) it is queued on @st->rx. So the
>> list is a mixture of both types of stream packets. So now that we
>> have all the helpers needed we can wire their processing up. But
>> since virNetClientStreamRecvPacket doesn't support
>> VIR_STREAM_RECV_STOP_AT_HOLE flag yet, let's turn all received
>> skips into zeroes repeating requested times.
>>
> 
> Up to this point - I thought I had a good idea of what was going on, but
> this patch loses me.
> 
> I thought there was an "ordered" message queue to be processed...
> ordered in so much as we're "reading" a file and sending either a 'data'
> segment w/ data or a 'skip' segment with some number of bytes to skip.
> Where it could be:
> 
>start...skip...data...[skip...data...]end
>start...data...skip...[data...skip...]end
>start...data...end
>start...skip...end
> 
> So why does the code process and sum up the skips?

Because our streams are slightly more generic than just a file transfer.
I mean, we use it for just any data transfer. Therefore it could be used
even for this

start -> data -> skip -> skip -> data -> end.

You won't have this with regular files stored on a disk, but then again,
virStream can be used (and is used) for generic data transfer.
vol-download an vol-upload is just a subset of its usage.

Now, with that example, it makes sense to merge those skips into one
big, doesn't it? But I agree, it looks a bit weird. I can drop the
merging - it's not a performance issue. I was just trying to save caller
couple of calls to our APIs.

> 
> Is this because of some implementation detail (that I already forgot) of
> the message queue where signals are done after each "data..." or
> "skip...", so it won't matter?
> 
> Why not have everything you need in place before you wire this up -
> patch 25 and 30 seem to be able to go after "most of" patch 31.
> 
> John
> 
> I'm going to stop here.

Fair enough. I'm gonna send v2 with most of your review points worked
in. Except, for now I'm still gonna use Skip() and HoleSize() - changing
that now would be a non-trivial piece of work and thus I'd like to do it
just once, when we have clear agreement on naming. Hope you understand that.

> 
>> Signed-off-by: Michal Privoznik 
>> ---
>>  src/rpc/virnetclientstream.c | 44 
>> ++--
>>  1 file changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
>> index c773524..ff35137 100644
>> --- a/src/rpc/virnetclientstream.c
>> +++ b/src/rpc/virnetclientstream.c
>> @@ -296,6 +296,8 @@ int virNetClientStreamQueuePacket(virNetClientStreamPtr 
>> st,
>>  
>>  virObjectLock(st);
>>  
>> +/* Don't distinguish VIR_NET_STREAM and VIR_NET_STREAM_SKIP
>> + * here just yet. We want in order processing! */
>>  virNetMessageQueuePush(>rx, tmp_msg);
>>  
>>  virNetClientStreamEventTimerUpdate(st);
>> @@ -359,7 +361,7 @@ int virNetClientStreamSendPacket(virNetClientStreamPtr 
>> st,
>>  }
>>  
>>  
>> -static int ATTRIBUTE_UNUSED
>> +static int
>>  virNetClientStreamHandleSkip(virNetClientPtr client,
>>   virNetClientStreamPtr st)
>>  {
>> @@ -435,6 +437,8 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr 
>> st,
>>  virCheckFlags(0, -1);
>>  
>>  virObjectLock(st);
>> +
>> + reread:
>>  if (!st->rx && !st->incomingEOF) {
>>  virNetMessagePtr msg;
>>  int ret;
>> @@ -466,8 +470,44 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr 
>> st,
>>  }
>>  
>>  VIR_DEBUG("After IO rx=%p", st->rx);
>> +
>> +while (st->rx &&
>> +   st->rx->header.type == VIR_NET_STREAM_SKIP) {
>> +/* Handle skip sent to us by server. */
>> +
>> +if (virNetClientStreamHandleSkip(client, st) < 0)
>> +goto cleanup;

You can see my reasoning from above in effect here.

>> +}
>> +
>> +if (!st->rx && !st->incomingEOF && !st->skipLength) {
>> +if (nonblock) {
>> +VIR_DEBUG("Non-blocking mode and no data available");
>> +rv = -2;
>> +goto cleanup;
>> +}
>> +
>> +/* We have consumed all packets from incoming queue but those
>> + * were only skip packets, no data. Read the stream again. */
>> +goto reread;
>> +}
>> +
>>  want = nbytes;
>> -while (want && st->rx) {
>> +
>> +if (st->skipLength) {
>> +/* Pretend skipLength zeroes was read from stream. */
>> +size_t len = want;
>> +
>> +if (len > st->skipLength)
>> +len = st->skipLength;
>> +
>> +memset(data, 0, len);
>> +st->skipLength -= len;
>> +want -= len;
>> +}
>> +
>> +while (want &&
>> +   st->rx &&
>> +   

Re: [libvirt] RFE: virsh list improvement (--description and --details)

[Pavel Hrdina]

On Mon, May 08, 2017 at 09:16:26AM +0200, Przemysław Sztoch wrote:
> Dear Michal and Pavel,
> 
> We cover about 100 clients who have their own and simple CentOS KVM 
> installations.
> Their technical skills are far from writing python scripts. They expect 
> simple solutions.

Libvirt is an library that provides unified API to manage different
hypervisors.  Virsh is a simple command line tool to manage one host,
it's not an advanced management application.

> Talking to our helpdesk, I found that 70% of libvirt and virtualization 
> problems are:
> A) lack of autostart activation on critical guests; then occasional 
> failures and reboots affect lack of automatic startup of key services,
> B) frequent overcommiting of allocated virtual processors and memory due 
> to the lack of basic planning and addition operations of local admin 
> stuff :-(,
> C) misconfiguration of qemu-agent, which affects many problems with safe 
> restart, snapshot, backup, etc. (the "Time" column is a perfect 
> diagnostic here)
> D) leaving unnecessary snapshots that lie unused after many months,
> E) live migration attempts that fail to put domain in a transient mode 
> leave the guests disappearing in unexplained circumstances after kvm 
> host restart :-)
> 
> Virtually all the above problems of everyday life, our helpdesk is now 
> able to diagnose by command:
> virsh list --details --managed-save
> By the way, they can easily update the documentation with one compact list.
> 
> I do not understand your dislike for the proposed changes. All the 
> members of our team and teams of our partners have been very 
> enthusiastic about the new functionality.
> You govern, so you have to decide. ;-)

My dislike is that virsh isn't a management application, it's more like
a shell for virtualization.  All the listed problems are things, that
should be handled by management application build on top of libvirt.
This is a common misuse of libvirt.

For simple workstation usage there is virt-manager.  For advanced
management tasks there are different type of applications, see
.

To address the patch itself, as it was already stated by Peter, instead
of adding --details there is command line tool "virt-top" and adding
--description isn't a good idea, because it will break the simple table
printed by "virsh list".  Abusing "virsh list" to provide statistics and
detailed information about domains isn't a way to go.

Pavel

> 
> Przemyslaw Sztoch

> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 0ca53e4..1c3ec37 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1769,6 +1769,14 @@ static const vshCmdOptDef opts_list[] = {
>   .type = VSH_OT_BOOL,
>   .help = N_("show domain title")
>  },
> +{.name = "description",
> + .type = VSH_OT_BOOL,
> + .help = N_("show domain description")
> +},
> +{.name = "details",
> + .type = VSH_OT_BOOL,
> + .help = N_("show domain details")
> +},
>  {.name = NULL}
>  };
>  
> @@ -1780,6 +1788,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>  {
>  bool managed = vshCommandOptBool(cmd, "managed-save");
>  bool optTitle = vshCommandOptBool(cmd, "title");
> +bool optDescription = vshCommandOptBool(cmd, "description");
> +bool optDetails = vshCommandOptBool(cmd, "details");
>  bool optTable = vshCommandOptBool(cmd, "table");
>  bool optUUID = vshCommandOptBool(cmd, "uuid");
>  bool optName = vshCommandOptBool(cmd, "name");
> @@ -1822,6 +1832,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>  
>  VSH_EXCLUSIVE_OPTIONS("table", "name");
>  VSH_EXCLUSIVE_OPTIONS("table", "uuid");
> +VSH_EXCLUSIVE_OPTIONS("description", "title");
> +VSH_EXCLUSIVE_OPTIONS("details", "title");
> +VSH_EXCLUSIVE_OPTIONS("details", "description");
>  
>  if (!optUUID && !optName)
>  optTable = true;
> @@ -1831,9 +1844,19 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>  
>  /* print table header in legacy mode */
>  if (optTable) {
> -if (optTitle)
> +if (optTitle || optDescription)
>  vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n",
> -  _("Id"), _("Name"), _("State"), _("Title"),
> +  _("Id"), _("Name"), _("State"),
> +  optTitle ? _("Title") : _("Description"),
> +  "-"
> +  "-");
> +else if (optDetails)
> +vshPrintExtra(ctl, " %-5s %-30s %-10s %-13s %-13s %5s %9s %9s 
> %s\n%s\n",
> +  _("Id"), _("Name"), _("State"), 
> +  _("Autostart"), _("Persistent"),
> +  _("vCPU"), _("Memory"), _("Snapshots"),
> +  _("Time"),
> +  

Re: [libvirt] GSoC Project

[Cedric Bosdonnat]

Hi Venkat,

Congratulations for being selected! I look forward to count you as
a new regular contributor of the libvirt community.

--
Cedric

On Sat, 2017-05-06 at 09:01 +0530, Venkat Datta wrote:
> Hi Team,
> 
> I'm an undergraduate student studying Computer Science at PES University , 
> India . 
> I had applied for the GSoC program this year . I'm happy to hear that my 
> proposal has been accepted. 
> The project idea i will be working on " Conversion to and from OCI-formatted 
> containers " 
> [ 
> http://wiki.libvirt.org/page/Google_Summer_of_Code_Ideas#Conversion_to_and_from_OCI-formatted_containers
>  ].
> 
> I'm looking forward to learn more and keep contributing to the libvirt 
> community.
> 
> Thanks,
> Venkat Datta N H
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

[libvirt] [PATCH] Post-release version bump to 3.4.0

[Erik Skultety]

Signed-off-by: Erik Skultety 
---

Pushed as trivial.

 configure.ac  | 2 +-
 docs/news.xml | 8 
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 0b564a030..3329b1781 100644
--- a/configure.ac
+++ b/configure.ac
@@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser General 
Public
 dnl License along with this library.  If not, see
 dnl .

-AC_INIT([libvirt], [3.3.0], [libvir-list@redhat.com], [], [http://libvirt.org])
+AC_INIT([libvirt], [3.4.0], [libvir-list@redhat.com], [], [http://libvirt.org])
 AC_CONFIG_SRCDIR([src/libvirt.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/docs/news.xml b/docs/news.xml
index fb4e792ec..2f0144972 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -33,6 +33,14 @@
  -->

 
+  
+
+
+
+
+
+
+  
   
 
   
--
2.12.2

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