Re: [libvirt] [PATCHv2 6/6] qemu: format caching-mode on iommu command line

[John Ferlan]

On 04/20/2017 08:19 AM, Ján Tomko wrote:
> Format the caching-mode option for the intel-iommu device,
> based on its  attribute value.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  src/qemu/qemu_capabilities.c  |  2 ++
>  src/qemu/qemu_capabilities.h  |  1 +
>  src/qemu/qemu_command.c   | 11 +++
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml  |  1 +
>  .../qemuxml2argv-intel-iommu-caching.args | 19 
> +++
>  tests/qemuxml2argvtest.c  |  5 +
>  6 files changed, 39 insertions(+)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 0c7d7b1..88ea306 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -366,6 +366,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>"query-named-block-nodes",
>"kernel-irqchip",
>"intel-iommu-intremap",

This is the 255th capability so it'll need separation like every other
5th element with a comment after the ",  /* 255 */

> +  "intel-iommu-caching",
>  );
>  
>  
> @@ -1718,6 +1719,7 @@ static struct virQEMUCapsStringFlags 
> virQEMUCapsObjectPropsUSBNECXHCI[] = {
>  
>  static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = {
>  { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP },
> +{ "caching-mode", QEMU_CAPS_INTEL_IOMMU_CACHING },
>  };
>  
>  /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format 
> */
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 4d19691..2409377 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -403,6 +403,7 @@ typedef enum {
>  QEMU_CAPS_QUERY_NAMED_BLOCK_NODES, /* qmp query-named-block-nodes */
>  QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */
>  QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */

Likewise,

/* 255 */

Needs to be added.

> +QEMU_CAPS_INTEL_IOMMU_CACHING, /* intel-iommu.caching-mode */
>  
>  QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1f97bd0..55800c7 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6690,6 +6690,13 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
>   "with this QEMU binary"));
>  return -1;
>  }
> +if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT &&
> +!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_CACHING))  {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("iommu: caching mode is not supported "
> + "with this QEMU binary"));
> +return -1;
> +}
>  break;
>  case VIR_DOMAIN_IOMMU_MODEL_LAST:
>  break;
> @@ -6719,6 +6726,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
>  virBufferAsprintf(, ",intremap=%s",
>
> virTristateSwitchTypeToString(iommu->intremap));
>  }
> +if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) {
> +virBufferAsprintf(, ",caching-mode=%s",
> +  virTristateSwitchTypeToString(iommu->caching));
> +}
>  case VIR_DOMAIN_IOMMU_MODEL_LAST:
>  break;
>  }
> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml 
> b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> index 799c52a..0193492 100644
> --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> @@ -209,6 +209,7 @@
>
>
>
> +  
>2008090
>0
> (v2.9.0-rc0-142-g940a8ce)
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args 
> b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args
> new file mode 100644
> index 000..dee63f4
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args
> @@ -0,0 +1,19 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-x86_64 \
> +-name QEMUGuest1 \
> +-S \
> +-machine q35,accel=tcg \
> +-m 214 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot c \
> +-device intel-iommu,intremap=on,caching-mode=on

Hmm so this proves that kernel-irqchip=split (or =on) isn't required
then?

It's just not clear, so I'm wondering.

Could be adjustments to the .args. based on patch5, but I think they're
obvious...

ACK with at least the comments added like other 

Re: [libvirt] [PATCHv2 5/6] conf: add caching attribute to iommu device

[John Ferlan]

On 04/20/2017 08:19 AM, Ján Tomko wrote:
> Add a new attribute to control the caching mode.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  docs/formatdomain.html.in  |  9 +++
>  docs/schemas/domaincommon.rng  |  5 
>  src/conf/domain_conf.c | 24 ---
>  src/conf/domain_conf.h |  1 +
>  .../qemuxml2argv-intel-iommu-caching.xml   | 28 
> ++
>  .../qemuxml2xmlout-intel-iommu-caching.xml |  1 +
>  tests/qemuxml2xmltest.c|  1 +
>  7 files changed, 66 insertions(+), 3 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index f5a8e76..dbca316 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7364,6 +7364,15 @@ qemu-kvm -net nic,model=? /dev/null
>(QEMU only)
>  
>
> +  caching
> +  
> +
> +  The caching attribute with possible values
> +  on and off can be used to
> +  turn on the caching mode. Since 
> 3.3.0
> +  (QEMU only)

Useful for what?  Required for what?  A bit more description about the
relationship with intremap might be nice as well.

Is there a relationship with the irqmode settings?  If so, then I would
think there should be checks in domain post processing that ensure that
if this is on, then the irqmode setting is adjusted appropriately.


> +
> +  
>  
>
>  



> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml 
> b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml
> new file mode 100644
> index 000..bab4fcb
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml
> @@ -0,0 +1,28 @@
> +
> +  QEMUGuest1
> +  c7a5fdbd-edaf-9455-926a-d65c16db1809
> +  219100
> +  219100
> +  1
> +  
> +hvm
> +
> +  
> +  
> +  destroy
> +  restart
> +  destroy
> +  
> +/usr/bin/qemu-system-x86_64
> +
> +
> +   function='0x2'/>
> +
> +
> +
> +
> +
> +  
> +
> +  
> +

Maybe this is the XML that gets more devices added that prove the
ordering is correct.  IDC which one, but we should "ensure" the ordering
is right..

John

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

Re: [libvirt] [PATCHv2 4/6] qemu: format intremap= on intel-iommu command line

[John Ferlan]

On 04/20/2017 08:19 AM, Ján Tomko wrote:
> Add the intremap= option to QEMU command line, corresponding
> to the  attribute of the iommu device.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  src/qemu/qemu_capabilities.c   |  8 
>  src/qemu/qemu_capabilities.h   |  1 +
>  src/qemu/qemu_command.c| 18 
>  .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 22 +++---
>  .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 24 +++
>  .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 24 +++
>  .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 28 +
>  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |  1 +
>  .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 37 
>  tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |  1 +
>  .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 49 
> ++
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |  1 +
>  .../qemuxml2argv-intel-iommu-irqchip.args  |  2 +-
>  tests/qemuxml2argvtest.c   |  1 +
>  14 files changed, 173 insertions(+), 44 deletions(-)
> 

ACK -

John

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

Re: [libvirt] [PATCHv2 3/6] conf: add to

[John Ferlan]

On 04/20/2017 08:19 AM, Ján Tomko wrote:
> Add a new attribute to control interrupt remapping.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  docs/formatdomain.html.in  | 22 -
>  docs/schemas/domaincommon.rng  |  9 +
>  src/conf/domain_conf.c | 38 
> +++---
>  src/conf/domain_conf.h |  1 +
>  .../qemuxml2argv-intel-iommu-irqchip.xml   |  4 ++-
>  5 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index abf089a..f5a8e76 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7335,7 +7335,9 @@ qemu-kvm -net nic,model=? /dev/null
>  
>  ...
>  devices
> -  iommu model='intel'/
> +  iommu model='intel'
> +driver intremap='on'/
> +  /iommu
>  /devices
>  ...
>  
> @@ -7346,6 +7348,24 @@ qemu-kvm -net nic,model=? /dev/null
>Currently only the intel model is supported.
>  
>
> +  driver
> +  
> +
> +  The driver subelement can be used to configure
> +  additional options:
> +
> +
> +  intremap
> +  
> +
> +  The intremap attribute with possible values
> +  on and off can be used to
> +  turn on interrupt remapping. Since 
> 3.3.0
> +  (QEMU only)

It seems there is a relationship between this parameter and irqchip
mode? IOW: Is it a requirement that irqchip be "split" or "on"? It's
difficult to ascertain from the bz, but if so, then I'd expect a domain
conf post processing in this patch.

Reading the bz it seems though that this parameter is optional for
"certain" devices; however, for "general emulated devices" it's what is
used to enable VT-d protection, but that could also be read as if
'intel-iommu' is enabled, then VT-d protection is on by default with
'intremap' just having some other affect.

Is there something we could describe here at a very high level to
describe the usefulness/need for this parameter?

John

> +
> +  
> +
> +  
>  
>  
>  Security label
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index df3143e..7930d85 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3889,6 +3889,15 @@
>
>  intel
>
> +  
> +
> +  
> +
> +  
> +
> +  
> +
> +  
>  
>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 931320e..d40d129 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14057,12 +14057,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
>  
>  
>  static virDomainIOMMUDefPtr
> -virDomainIOMMUDefParseXML(xmlNodePtr node)
> +virDomainIOMMUDefParseXML(xmlNodePtr node,
> +  xmlXPathContextPtr ctxt)
>  {
>  virDomainIOMMUDefPtr iommu = NULL, ret = NULL;
> +xmlNodePtr save = ctxt->node;
>  char *tmp = NULL;
>  int val;
>  
> +ctxt->node = node;
> +
>  if (VIR_ALLOC(iommu) < 0)
>  goto cleanup;
>  
> @@ -14079,10 +14083,20 @@ virDomainIOMMUDefParseXML(xmlNodePtr node)
>  
>  iommu->model = val;
>  
> +VIR_FREE(tmp);
> +if ((tmp = virXPathString("string(./driver/@intremap)", ctxt))) {
> +if ((val = virTristateSwitchTypeFromString(tmp)) < 0) {
> +virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: 
> %s"), tmp);
> +goto cleanup;
> +}
> +iommu->intremap = val;
> +}
> +
>  ret = iommu;
>  iommu = NULL;
>  
>   cleanup:
> +ctxt->node = save;
>  VIR_FREE(iommu);
>  VIR_FREE(tmp);
>  return ret;
> @@ -14235,7 +14249,7 @@ virDomainDeviceDefParse(const char *xmlStr,
>  goto error;
>  break;
>  case VIR_DOMAIN_DEVICE_IOMMU:
> -if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node)))
> +if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt)))
>  goto error;
>  break;
>  case VIR_DOMAIN_DEVICE_NONE:
> @@ -18365,7 +18379,7 @@ virDomainDefParseXML(xmlDocPtr xml,
>  }
>  
>  if (n > 0) {
> -if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0])))
> +if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0], ctxt)))
>  goto error;
>  }
>  VIR_FREE(nodes);
> @@ -24019,8 +24033,24 @@ static void
>  virDomainIOMMUDefFormat(virBufferPtr buf,
>  const virDomainIOMMUDef *iommu)
>  {
> -virBufferAsprintf(buf, "\n",
> +virBuffer childBuf = VIR_BUFFER_INITIALIZER;
> +
> +virBufferAdjustIndent(, virBufferGetIndent(buf, false) + 2);
> +
> +if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) {
> +virBufferAsprintf(, "\n",
> +  

Re: [libvirt] [PATCHv2 2/6] qemu: format kernel_irqchip on the command line

[John Ferlan]

On 04/20/2017 08:19 AM, Ján Tomko wrote:
> Add kernel_irqchip=off/split/on to the QEMU command line
> and a capability that looks for it in query-command-line-options
> output.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  src/libvirt_private.syms  |  1 +
>  src/qemu/qemu_capabilities.c  |  2 ++
>  src/qemu/qemu_capabilities.h  |  1 +
>  src/qemu/qemu_command.c   | 11 +++
>  tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml  |  1 +
>  .../qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml |  1 +
>  .../qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml |  1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml |  1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml   |  1 +
>  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml   |  1 +
>  tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml  |  1 +
>  .../qemuxml2argv-intel-iommu-irqchip.args | 19 
> +++
>  tests/qemuxml2argvtest.c  |  4 
>  21 files changed, 53 insertions(+)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args
> 

ACK,

John

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

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

[John Ferlan]

On 04/20/2017 08:19 AM, Ján Tomko wrote:
> Add a new  element with a mode attribute.
> 
> Possible values are off, split or on.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  docs/formatdomain.html.in  | 10 +++
>  docs/schemas/domaincommon.rng  | 16 ++
>  src/conf/domain_conf.c | 34 
> +-
>  src/conf/domain_conf.h | 12 
>  .../qemuxml2argv-intel-iommu-irqchip.xml   | 29 ++
>  .../qemuxml2xmlout-intel-iommu-irqchip.xml |  1 +
>  tests/qemuxml2xmltest.c|  1 +
>  7 files changed, 102 insertions(+), 1 deletion(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index b1e38f0..abf089a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1643,6 +1643,7 @@
>/kvm
>pvspinlock state='on'/
>gic version='2'/
> +  irqchip mode='split'/
>  
>  /features
>  ...
> @@ -1804,6 +1805,15 @@
>for hypervisor to decide.
>Since 2.1.0
>
> +  irqchip
> +  Tune the in-kernel irqchip. Possible values for the
> +  mode attribute are:
> +  on, split and off.
> +  split is useful for using interrupt remapping
> +  with the IOMMU device.

Something that isn't implemented until the subsequent patch, but I'm not
against describing this feature a bit more here...

I think most importantly what setting this feature will "do" would be
useful.  How does someone know they need this? And secondarily what
would it be required for? What does "on" really do?  IOW: What the
difference between split and on.

> +  The default is left for hypervisor to decide.
> +  Since 3.3.0 (QEMU only)
> +  
>  
>  
>  Time keeping

...

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml 
> b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
> new file mode 100644
> index 000..cc895af
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
> @@ -0,0 +1,29 @@
> +
> +  QEMUGuest1
> +  c7a5fdbd-edaf-9455-926a-d65c16db1809
> +  219100
> +  219100
> +  1
> +  
> +hvm
> +
> +  
> +  
> +
> +  
> +  
> +  destroy
> +  restart
> +  destroy
> +  
> +/usr/bin/qemu-system-x86_64
> +
> +
> +   function='0x2'/>
> +
> +
> +
> +
> +
> +  
> +

Could a few devices be added so that future patches will actually
generate a command line that would should the code conforms to the
requirement that the "-device intel-iommu" appears in the command list
before the "rest of the devices".

John

> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml 
> b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
> new file mode 12
> index 000..58a0199
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
> @@ -0,0 +1 @@
> +../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml
> \ No newline at end of file
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index e4b510f..c7d4788 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1121,6 +1121,7 @@ mymain(void)
>  DO_TEST("intel-iommu-machine",
>  QEMU_CAPS_MACHINE_OPT,
>  QEMU_CAPS_MACHINE_IOMMU);
> +DO_TEST("intel-iommu-irqchip", NONE);
>  
>  DO_TEST("cpu-check-none", NONE);
>  DO_TEST("cpu-check-partial", NONE);
> 

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

Re: [libvirt] [PATCH v3] Add support for Veritas HyperScale (VxHS) block device protocol

[ashish mittal]

On Tue, Apr 11, 2017 at 3:47 PM, John Ferlan  wrote:
>
>
> On 04/10/2017 07:32 PM, ashish mittal wrote:
>> Hi,
>>
>> I'm trying to figure out what changes are needed in the libvirt vxhs
>> patch to support passing TLS X509 arguments to qemu, similar to the
>> following -
>>
>> Sample QEMU command line passing TLS credentials to the VxHS block
>> device (run in secure mode):
>> ./qemu-io --object
>> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
>> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "",
>> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
>>
>> I was hoping to find some NBD code related to this, but not able to
>> locate it. Any pointers will be appreciated.
>
> Well you have a couple of things to deal with... There's the creation of
> the TLS object and there's altering the parameters used for the qemu
> command based on your needs/model.
>
> First off you'll need to figure out where/how you're going to define
> where the TLS creds exist. For that, I suspect you'll have code similar
> to how chardevTLS support was added.  Essentially some way to either use
> an existing TLS environment or a way to allow someone to define a vxhs
> specific environment (hint, see src/qemu/qemu_conf.c, src/qemu/qemu.conf
> - I've made changes recently there too).
>
> For the TLS object creation on the command line, see
> qemuBuildTLSx509CommandLine to see how the code builds the
> "tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client" portion
> of your command line.
>
> I forget if hot plug was in your plan, but see qemuDomainGetTLSObjects,
> qemuDomainAddTLSObjects, and qemuDomainDelTLSObjects for that.
>
> The rest of the command line is going to be a bit tricky since using the
> "newer" driver syntax for libvirt is "sparse". Traditionally libvirt has
> used "-drive file=[$uri:]$path,format=$driver,..."  (use grep "\-drive
> file" tests/*/*.args from a libvirt git directory - you can grep that
> output for gluster or rbd to see the uri format).
>
> IIUC the qemu changes correctly though, you cannot use that "file="
> syntax, instead you'll need to format the command line similar to how
> things were done for gluster to add multiple host support where the
> syntax is "-drive 'file.driver=gluster,file.volume=..." (use grep
> "\-drive file.driver" tests/*/*.args to see how this is done for gluster).
>

Hi! Thanks for explaining how TLS could work for block devices in
general, and specifically for VxHS. I agree that the plan should be to
first have the base VxHS patch ready (sans TLS support), and then
build the TLS support in a series of patches on top of the base patch.
To that end, I have made changes to v3 to fix the problems pointed out
in this series and plan to submit the new patch soon. I do have some
confusion regarding your comment above - I thought I have already
implemented the new syntax similar to gluster's "-drive
'file.driver=gluster,file.volume=..." in this v3 patch series.

>>> v3 changelog:
>>> (1) Implemented the modern syntax for VxHS disk specification.
>>> (2) Changed qemuxml2argvdata VxHS test case to verify the new syntax.

Please see the newly added xml2argv test +++
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args

+-drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
+file.server.host=192.168.0.1,file.server.port=,format=raw,if=none,\

Let me know if I'm still missing something on the new syntax. Thanks!

> That code/support was added in a series starting at commit id '22ad4a7c'
> and working your way forward through about 18 patches. Using a visual
> tool like gitk helps a lot...
>
> I think what will be easiest is to start at that commit and look "up"
> for gluster specific changes.  Be careful not to fully cut-n-paste
> because there have been patches since that time to fix some issues with
> the initial implementation. I point it out only as a way for you to see
> which modules and where "similar" code exists.
>
> You'll also note there is an nbd patch in that series of patches - not
> sure how much that helps, but it perhaps gives you some amount of
> guidelines. Although I don't believe nbd was added to the command line -
> it was just a way of syntax generation/testing.
>
>
> John
>
>>
>> Thanks,
>> Ashish
>>
>> On Wed, Feb 1, 2017 at 8:36 AM, John Ferlan  wrote:
>>> [...]
>>> Pressed send too soon, sigh.
>>>
>>>
>>
>> #1. Based on Peter's v2 comments, we don't want to support the
>> older/legacy syntax for VxHS, so it's something that should be removed -
>> although we should check for it being present and fail if found.
>>
>
> I am testing with changed code to return error if legacy syntax is
> found for VxHS. Also added a test case to check for failure on legacy
> syntax and it seems to pass (test #41 below).
>
> Then I added a pass test case to check conversion from new native

[libvirt] [PATCH 10/15] nwfilter: Replace virNWFilterConfigFile with virFileBuildPath

[John Ferlan]

Remove open coded helper.

Signed-off-by: John Ferlan 
---
 src/conf/nwfilter_conf.c | 13 +
 src/conf/nwfilter_conf.h |  4 
 2 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index b69d9db..2352e60 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2775,7 +2775,7 @@ virNWFilterSaveXML(const char *configDir,
 char *configFile = NULL;
 int ret = -1;
 
-if ((configFile = virNWFilterConfigFile(configDir, def->name)) == NULL)
+if (!(configFile = virFileBuildPath(configDir, def->name, ".xml")))
 goto cleanup;
 
 if (virFileMakePath(configDir) < 0) {
@@ -3236,17 +3236,6 @@ virNWFilterDefFormat(const virNWFilterDef *def)
 }
 
 
-char *
-virNWFilterConfigFile(const char *dir,
-  const char *name)
-{
-char *ret = NULL;
-
-ignore_value(virAsprintf(, "%s/%s.xml", dir, name));
-return ret;
-}
-
-
 int
 virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB,
  void *opaque)
diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index d8a5daf..5bf9c3d 100644
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -593,10 +593,6 @@ int
 virNWFilterSaveConfig(const char *configDir,
   virNWFilterDefPtr def);
 
-char *
-virNWFilterConfigFile(const char *dir,
-  const char *name);
-
 virNWFilterDefPtr
 virNWFilterDefParseString(const char *xml);
 
-- 
2.9.3

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

[libvirt] [PATCH 14/15] nwfilter: Move save of config until after successful assign

[John Ferlan]

Only save the config when using a generated UUID if we were able to
create an object for the def. There could have been "other reasons"
for the assignment to fail, so saving the config could be incorrect.

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index ec28b05..e37acf0 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -510,14 +510,14 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr 
nwfilters,
 goto error;
 }
 
+if (!(obj = virNWFilterObjListAssignDef(nwfilters, def)))
+goto error;
+
 /* We generated a UUID, make it permanent by saving the config to disk */
 if (!def->uuid_specified &&
 virNWFilterSaveConfig(configFile, def) < 0)
 goto error;
 
-if (!(obj = virNWFilterObjListAssignDef(nwfilters, def)))
-goto error;
-
 VIR_FREE(configFile);
 return obj;
 
-- 
2.9.3

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

[libvirt] [PATCH 09/15] nwfilter: Make a common UUID lookup function from driver

[John Ferlan]

Rather than separate calls, use a common call and generate a better
error message which includes the incorrect uuidstr

Signed-off-by: John Ferlan 
---
 src/nwfilter/nwfilter_driver.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 650c528..781a7a0 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -356,6 +356,21 @@ nwfilterStateCleanup(void)
 }
 
 
+static virNWFilterObjPtr
+nwfilterObjFromNWFilter(const unsigned char *uuid)
+{
+virNWFilterObjPtr obj;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+if (!(obj = virNWFilterObjListFindByUUID(driver->nwfilters, uuid))) {
+virUUIDFormat(uuid, uuidstr);
+virReportError(VIR_ERR_NO_NWFILTER,
+   _("no nwfilter with matching uuid '%s'"), uuidstr);
+}
+return obj;
+}
+
+
 static virNWFilterPtr
 nwfilterLookupByUUID(virConnectPtr conn,
  const unsigned char *uuid)
@@ -365,14 +380,11 @@ nwfilterLookupByUUID(virConnectPtr conn,
 virNWFilterPtr ret = NULL;
 
 nwfilterDriverLock();
-obj = virNWFilterObjListFindByUUID(driver->nwfilters, uuid);
+obj = nwfilterObjFromNWFilter(uuid);
 nwfilterDriverUnlock();
 
-if (!obj) {
-virReportError(VIR_ERR_NO_NWFILTER,
-   "%s", _("no nwfilter with matching uuid"));
+if (!obj)
 goto cleanup;
-}
 def = virNWFilterObjGetDef(obj);
 
 if (virNWFilterLookupByUUIDEnsureACL(conn, def) < 0)
@@ -528,12 +540,8 @@ nwfilterUndefine(virNWFilterPtr nwfilter)
 virNWFilterWriteLockFilterUpdates();
 virNWFilterCallbackDriversLock();
 
-obj = virNWFilterObjListFindByUUID(driver->nwfilters, nwfilter->uuid);
-if (!obj) {
-virReportError(VIR_ERR_NO_NWFILTER,
-   "%s", _("no nwfilter with matching uuid"));
+if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid)))
 goto cleanup;
-}
 def = virNWFilterObjGetDef(obj);
 
 if (virNWFilterUndefineEnsureACL(nwfilter->conn, def) < 0)
@@ -575,14 +583,11 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter,
 virCheckFlags(0, NULL);
 
 nwfilterDriverLock();
-obj = virNWFilterObjListFindByUUID(driver->nwfilters, nwfilter->uuid);
+obj = nwfilterObjFromNWFilter(nwfilter->uuid);
 nwfilterDriverUnlock();
 
-if (!obj) {
-virReportError(VIR_ERR_NO_NWFILTER,
-   "%s", _("no nwfilter with matching uuid"));
+if (!obj)
 goto cleanup;
-}
 def = virNWFilterObjGetDef(obj);
 
 if (virNWFilterGetXMLDescEnsureACL(nwfilter->conn, def) < 0)
-- 
2.9.3

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

[libvirt] [PATCH 12/15] nwfilter: Move creation of configDir to driver initialization

[John Ferlan]

Rather than "wait" for the first config file to be created, force creation
of the configDir during driver state initialization.

Signed-off-by: John Ferlan 
---
 src/conf/nwfilter_conf.c   | 7 ---
 src/nwfilter/nwfilter_driver.c | 7 +++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 752d4e1..032700c 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2778,13 +2778,6 @@ virNWFilterSaveXML(const char *configDir,
 if (!(configFile = virFileBuildPath(configDir, def->name, ".xml")))
 goto cleanup;
 
-if (virFileMakePath(configDir) < 0) {
-virReportSystemError(errno,
- _("cannot create config directory '%s'"),
- configDir);
-goto cleanup;
-}
-
 virUUIDFormat(def->uuid, uuidstr);
 ret = virXMLSaveFile(configFile,
  virXMLPickShellSafeComment(def->name, uuidstr),
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index faa4fe8..5e62023 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -40,6 +40,7 @@
 #include "nwfilter_driver.h"
 #include "nwfilter_gentech_driver.h"
 #include "configmake.h"
+#include "virfile.h"
 #include "virstring.h"
 #include "viraccessapicheck.h"
 
@@ -237,6 +238,12 @@ nwfilterStateInitialize(bool privileged,
 
 VIR_FREE(base);
 
+if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) {
+virReportSystemError(errno, _("cannot create config directory '%s'"),
+ driver->configDir);
+goto error;
+}
+
 if (!(driver->nwfilters = virNWFilterObjListNew()))
 goto error;
 
-- 
2.9.3

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

[libvirt] [PATCH 13/15] nwfilter: Move configFile name processing into driver

[John Ferlan]

In preparation for it becoming part of the nwfilter object, move all the
processing of the generation of the configFile name into the driver code.

Signed-off-by: John Ferlan 
---
 src/conf/nwfilter_conf.c   | 36 +---
 src/conf/nwfilter_conf.h   |  6 +++---
 src/conf/virnwfilterobj.c  |  2 +-
 src/nwfilter/nwfilter_driver.c | 14 --
 4 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 032700c..6b67c3a 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2767,30 +2767,21 @@ virNWFilterDefParseFile(const char *filename)
 
 
 int
-virNWFilterSaveXML(const char *configDir,
+virNWFilterSaveXML(const char *configFile,
virNWFilterDefPtr def,
const char *xml)
 {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
-char *configFile = NULL;
-int ret = -1;
-
-if (!(configFile = virFileBuildPath(configDir, def->name, ".xml")))
-goto cleanup;
 
 virUUIDFormat(def->uuid, uuidstr);
-ret = virXMLSaveFile(configFile,
- virXMLPickShellSafeComment(def->name, uuidstr),
- "nwfilter-edit", xml);
-
- cleanup:
-VIR_FREE(configFile);
-return ret;
+return virXMLSaveFile(configFile,
+  virXMLPickShellSafeComment(def->name, uuidstr),
+  "nwfilter-edit", xml);
 }
 
 
 int
-virNWFilterSaveConfig(const char *configDir,
+virNWFilterSaveConfig(const char *configFile,
   virNWFilterDefPtr def)
 {
 int ret = -1;
@@ -2799,7 +2790,7 @@ virNWFilterSaveConfig(const char *configDir,
 if (!(xml = virNWFilterDefFormat(def)))
 goto cleanup;
 
-if (virNWFilterSaveXML(configDir, def, xml) < 0)
+if (virNWFilterSaveXML(configFile, def, xml) < 0)
 goto cleanup;
 
 ret = 0;
@@ -2925,26 +2916,17 @@ virNWFilterTriggerVMFilterRebuild(void)
 
 
 int
-virNWFilterDeleteDef(const char *configDir,
+virNWFilterDeleteDef(const char *configFile,
  virNWFilterDefPtr def)
 {
-int ret = -1;
-char *configFile = NULL;
-
-if (!(configFile = virFileBuildPath(configDir, def->name, ".xml")))
-goto error;
-
 if (unlink(configFile) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot remove config for %s"),
def->name);
-goto error;
+return -1;
 }
 
-ret = 0;
- error:
-VIR_FREE(configFile);
-return ret;
+return  0;
 }
 
 
diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index 5cac260..5a3f008 100644
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -570,7 +570,7 @@ int
 virNWFilterTriggerVMFilterRebuild(void);
 
 int
-virNWFilterDeleteDef(const char *configDir,
+virNWFilterDeleteDef(const char *configFile,
  virNWFilterDefPtr def);
 
 virNWFilterDefPtr
@@ -581,12 +581,12 @@ char *
 virNWFilterDefFormat(const virNWFilterDef *def);
 
 int
-virNWFilterSaveXML(const char *configDir,
+virNWFilterSaveXML(const char *configFile,
virNWFilterDefPtr def,
const char *xml);
 
 int
-virNWFilterSaveConfig(const char *configDir,
+virNWFilterSaveConfig(const char *configFile,
   virNWFilterDefPtr def);
 
 virNWFilterDefPtr
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 7410047..ec28b05 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -512,7 +512,7 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr 
nwfilters,
 
 /* We generated a UUID, make it permanent by saving the config to disk */
 if (!def->uuid_specified &&
-virNWFilterSaveConfig(configDir, def) < 0)
+virNWFilterSaveConfig(configFile, def) < 0)
 goto error;
 
 if (!(obj = virNWFilterObjListAssignDef(nwfilters, def)))
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 5e62023..97d6952 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -495,6 +495,7 @@ nwfilterDefineXML(virConnectPtr conn,
 virNWFilterObjPtr obj = NULL;
 virNWFilterDefPtr objdef;
 virNWFilterPtr ret = NULL;
+char *configFile = NULL;
 
 if (!driver->privileged) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -512,12 +513,15 @@ nwfilterDefineXML(virConnectPtr conn,
 if (virNWFilterDefineXMLEnsureACL(conn, def) < 0)
 goto cleanup;
 
+if (!(configFile = virFileBuildPath(driver->configDir, def->name, ".xml")))
+goto cleanup;
+
 if (!(obj = virNWFilterObjListAssignDef(driver->nwfilters, def)))
 goto cleanup;
 def = NULL;
 objdef = virNWFilterObjGetDef(obj);
 
-if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) {
+if (virNWFilterSaveConfig(configFile, objdef) < 0) {
 

[libvirt] [PATCH 15/15] nwfilter: Move and rename virNWFilterSaveConfig

[John Ferlan]

Move into virnwfilterobj, rename the API to virNWFilterObjSaveConfig,
and reorder the arguments.

Signed-off-by: John Ferlan 
---
 src/conf/nwfilter_conf.c   | 20 
 src/conf/nwfilter_conf.h   |  4 
 src/conf/virnwfilterobj.c  | 23 ++-
 src/conf/virnwfilterobj.h  |  4 
 src/libvirt_private.syms   |  3 ++-
 src/nwfilter/nwfilter_driver.c |  2 +-
 6 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 6b67c3a..8925b2b 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2780,26 +2780,6 @@ virNWFilterSaveXML(const char *configFile,
 }
 
 
-int
-virNWFilterSaveConfig(const char *configFile,
-  virNWFilterDefPtr def)
-{
-int ret = -1;
-char *xml;
-
-if (!(xml = virNWFilterDefFormat(def)))
-goto cleanup;
-
-if (virNWFilterSaveXML(configFile, def, xml) < 0)
-goto cleanup;
-
-ret = 0;
- cleanup:
-VIR_FREE(xml);
-return ret;
-}
-
-
 int nCallbackDriver;
 #define MAX_CALLBACK_DRIVER 10
 static virNWFilterCallbackDriverPtr callbackDrvArray[MAX_CALLBACK_DRIVER];
diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index 5a3f008..2287689 100644
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -585,10 +585,6 @@ virNWFilterSaveXML(const char *configFile,
virNWFilterDefPtr def,
const char *xml);
 
-int
-virNWFilterSaveConfig(const char *configFile,
-  virNWFilterDefPtr def);
-
 virNWFilterDefPtr
 virNWFilterDefParseString(const char *xml);
 
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index e37acf0..898ca7f 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -487,6 +487,27 @@ virNWFilterObjListExport(virConnectPtr conn,
 }
 
 
+int
+virNWFilterObjSaveConfig(virNWFilterObjPtr obj,
+ const char *configFile)
+{
+virNWFilterDefPtr def = obj->def;
+int ret = -1;
+char *xml;
+
+if (!(xml = virNWFilterDefFormat(def)))
+goto cleanup;
+
+if (virNWFilterSaveXML(configFile, def, xml) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(xml);
+return ret;
+}
+
+
 static virNWFilterObjPtr
 virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters,
  const char *configDir,
@@ -515,7 +536,7 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr 
nwfilters,
 
 /* We generated a UUID, make it permanent by saving the config to disk */
 if (!def->uuid_specified &&
-virNWFilterSaveConfig(configFile, def) < 0)
+virNWFilterObjSaveConfig(obj, configFile) < 0)
 goto error;
 
 VIR_FREE(configFile);
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index 29d9086..f6a14de 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -98,6 +98,10 @@ virNWFilterObjListExport(virConnectPtr conn,
  virNWFilterObjListFilter aclfilter);
 
 int
+virNWFilterObjSaveConfig(virNWFilterObjPtr obj,
+ const char *configFile);
+
+int
 virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters,
  const char *configDir);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 170ecce..4b334a9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -740,7 +740,7 @@ virNWFilterRuleIsProtocolEthernet;
 virNWFilterRuleIsProtocolIPv4;
 virNWFilterRuleIsProtocolIPv6;
 virNWFilterRuleProtocolTypeToString;
-virNWFilterSaveConfig;
+virNWFilterSaveXML;
 virNWFilterTriggerVMFilterRebuild;
 virNWFilterUnlockFilterUpdates;
 virNWFilterUnRegisterCallbackDriver;
@@ -975,6 +975,7 @@ virNWFilterObjListNew;
 virNWFilterObjListNumOfNWFilters;
 virNWFilterObjListRemove;
 virNWFilterObjLock;
+virNWFilterObjSaveConfig;
 virNWFilterObjTestUnassignDef;
 virNWFilterObjUnlock;
 virNWFilterObjWantRemoved;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 97d6952..31e4867 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -521,7 +521,7 @@ nwfilterDefineXML(virConnectPtr conn,
 def = NULL;
 objdef = virNWFilterObjGetDef(obj);
 
-if (virNWFilterSaveConfig(configFile, objdef) < 0) {
+if (virNWFilterObjSaveConfig(obj, configFile) < 0) {
 virNWFilterObjListRemove(driver->nwfilters, obj);
 goto cleanup;
 }
-- 
2.9.3

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

[libvirt] [PATCH 11/15] nwfilter: Replace virNWFilterSaveDef with virNWFilterSaveConfig

[John Ferlan]

Essentially virNWFilterSaveDef executed in a different order the same
sequence of calls, so let's just make one point of reference.

Signed-off-by: John Ferlan 
---
 src/conf/nwfilter_conf.c   | 37 -
 src/conf/nwfilter_conf.h   |  4 
 src/libvirt_private.syms   |  2 +-
 src/nwfilter/nwfilter_driver.c |  2 +-
 4 files changed, 2 insertions(+), 43 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 2352e60..752d4e1 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2932,43 +2932,6 @@ virNWFilterTriggerVMFilterRebuild(void)
 
 
 int
-virNWFilterSaveDef(const char *configDir,
-   virNWFilterDefPtr def)
-{
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-char *xml;
-int ret = -1;
-char *configFile = NULL;
-
-if (virFileMakePath(configDir) < 0) {
-virReportSystemError(errno,
- _("cannot create config directory %s"),
- configDir);
-goto error;
-}
-
-if (!(configFile = virFileBuildPath(configDir, def->name, ".xml")))
-goto error;
-
-if (!(xml = virNWFilterDefFormat(def))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("failed to generate XML"));
-goto error;
-}
-
-virUUIDFormat(def->uuid, uuidstr);
-ret = virXMLSaveFile(configFile,
- virXMLPickShellSafeComment(def->name, uuidstr),
- "nwfilter-edit", xml);
-VIR_FREE(xml);
-
- error:
-VIR_FREE(configFile);
-return ret;
-}
-
-
-int
 virNWFilterDeleteDef(const char *configDir,
  virNWFilterDefPtr def)
 {
diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index 5bf9c3d..5cac260 100644
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -570,10 +570,6 @@ int
 virNWFilterTriggerVMFilterRebuild(void);
 
 int
-virNWFilterSaveDef(const char *configDir,
-   virNWFilterDefPtr def);
-
-int
 virNWFilterDeleteDef(const char *configDir,
  virNWFilterDefPtr def);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 13f2ab9..170ecce 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -740,7 +740,7 @@ virNWFilterRuleIsProtocolEthernet;
 virNWFilterRuleIsProtocolIPv4;
 virNWFilterRuleIsProtocolIPv6;
 virNWFilterRuleProtocolTypeToString;
-virNWFilterSaveDef;
+virNWFilterSaveConfig;
 virNWFilterTriggerVMFilterRebuild;
 virNWFilterUnlockFilterUpdates;
 virNWFilterUnRegisterCallbackDriver;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 781a7a0..faa4fe8 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -510,7 +510,7 @@ nwfilterDefineXML(virConnectPtr conn,
 def = NULL;
 objdef = virNWFilterObjGetDef(obj);
 
-if (virNWFilterSaveDef(driver->configDir, objdef) < 0) {
+if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) {
 virNWFilterObjListRemove(driver->nwfilters, obj);
 goto cleanup;
 }
-- 
2.9.3

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

[libvirt] [PATCH 07/15] nwfilter: Rename some virNWFilterObj* API's

[John Ferlan]

Prefix should have been virNWFilterObjList since the API is operating on
the list of filters

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c  | 76 +-
 src/conf/virnwfilterobj.h  | 36 
 src/libvirt_private.syms   | 14 +++
 src/nwfilter/nwfilter_driver.c | 22 +-
 src/nwfilter/nwfilter_gentech_driver.c | 12 +++---
 5 files changed, 80 insertions(+), 80 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index dcfd1e7..a259062 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -116,8 +116,8 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters)
 
 
 void
-virNWFilterObjRemove(virNWFilterObjListPtr nwfilters,
- virNWFilterObjPtr obj)
+virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters,
+ virNWFilterObjPtr obj)
 {
 size_t i;
 
@@ -138,8 +138,8 @@ virNWFilterObjRemove(virNWFilterObjListPtr nwfilters,
 
 
 virNWFilterObjPtr
-virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters,
- const unsigned char *uuid)
+virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters,
+ const unsigned char *uuid)
 {
 size_t i;
 virNWFilterObjPtr obj;
@@ -159,8 +159,8 @@ virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters,
 
 
 virNWFilterObjPtr
-virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters,
- const char *name)
+virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters,
+ const char *name)
 {
 size_t i;
 virNWFilterObjPtr obj;
@@ -180,9 +180,9 @@ virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters,
 
 
 static int
-_virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters,
- virNWFilterDefPtr def,
- const char *filtername)
+_virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters,
+ virNWFilterDefPtr def,
+ const char *filtername)
 {
 int rc = 0;
 size_t i;
@@ -202,12 +202,12 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr 
nwfilters,
 break;
 }
 
-obj = virNWFilterObjFindByName(nwfilters,
-   entry->include->filterref);
+obj = virNWFilterObjListFindByName(nwfilters,
+   entry->include->filterref);
 if (obj) {
 objdef = obj->def;
-rc = _virNWFilterObjDefLoopDetect(nwfilters, objdef,
-  filtername);
+rc = _virNWFilterObjListDefLoopDetect(nwfilters, objdef,
+  filtername);
 virNWFilterObjUnlock(obj);
 if (rc < 0)
 break;
@@ -220,7 +220,7 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr 
nwfilters,
 
 
 /*
- * virNWFilterObjDefLoopDetect:
+ * virNWFilterObjListDefLoopDetect:
  * @nwfilters : the nwfilters to search
  * @def : the filter definition that may add a loop and is to be tested
  *
@@ -230,10 +230,10 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr 
nwfilters,
  * Returns 0 in case no loop was detected, -1 otherwise.
  */
 static int
-virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters,
-virNWFilterDefPtr def)
+virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters,
+virNWFilterDefPtr def)
 {
-return _virNWFilterObjDefLoopDetect(nwfilters, def, def->name);
+return _virNWFilterObjListDefLoopDetect(nwfilters, def, def->name);
 }
 
 
@@ -286,13 +286,13 @@ virNWFilterDefEqual(const virNWFilterDef *def1,
 
 
 virNWFilterObjPtr
-virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
-virNWFilterDefPtr def)
+virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
+virNWFilterDefPtr def)
 {
 virNWFilterObjPtr obj;
 virNWFilterDefPtr objdef;
 
-if ((obj = virNWFilterObjFindByUUID(nwfilters, def->uuid))) {
+if ((obj = virNWFilterObjListFindByUUID(nwfilters, def->uuid))) {
 objdef = obj->def;
 
 if (STRNEQ(def->name, objdef->name)) {
@@ -305,7 +305,7 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 }
 virNWFilterObjUnlock(obj);
 } else {
-if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) {
+if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
 objdef = obj->def;
@@ -318,14 +318,14 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 }
 }
 
-if (virNWFilterObjDefLoopDetect(nwfilters, def) < 0) {
+if 

[libvirt] [PATCH 03/15] nwfilter: Remove unused 'active' in virNWFilterObj

[John Ferlan]

It was only ever set to false, which is ironically the default

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c | 1 -
 src/conf/virnwfilterobj.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 3c6bdbb..9e7d584 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -308,7 +308,6 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 return NULL;
 }
 virNWFilterObjLock(obj);
-obj->active = 0;
 
 if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs,
 nwfilters->count, obj) < 0) {
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index 2adffd9..7a2addf 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -29,7 +29,6 @@ typedef virNWFilterObj *virNWFilterObjPtr;
 struct _virNWFilterObj {
 virMutex lock;
 
-int active;
 int wantRemoved;
 
 virNWFilterDefPtr def;
-- 
2.9.3

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

[libvirt] [PATCH 04/15] nwfilter: Convert wantRemoved to bool

[John Ferlan]

It is what it is anyway, so let's describe it that way too.

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c | 4 ++--
 src/conf/virnwfilterobj.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 9e7d584..6e5c5b7 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -192,12 +192,12 @@ virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj)
 {
 int rc = 0;
 
-obj->wantRemoved = 1;
+obj->wantRemoved = true;
 /* trigger the update on VMs referencing the filter */
 if (virNWFilterTriggerVMFilterRebuild())
 rc = -1;
 
-obj->wantRemoved = 0;
+obj->wantRemoved = false;
 
 return rc;
 }
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index 7a2addf..f31938d 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -29,7 +29,7 @@ typedef virNWFilterObj *virNWFilterObjPtr;
 struct _virNWFilterObj {
 virMutex lock;
 
-int wantRemoved;
+bool wantRemoved;
 
 virNWFilterDefPtr def;
 virNWFilterDefPtr newDef;
-- 
2.9.3

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

[libvirt] [PATCH 08/15] nwfilter: Make _virNWFilterObjList private

[John Ferlan]

Move from virnwfilterobj.h to virnwfilterobj.c.

Create the virNWFilterObjListNew() API in order to allocate

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c  | 16 
 src/conf/virnwfilterobj.h  | 10 --
 src/libvirt_private.syms   |  1 +
 src/nwfilter/nwfilter_driver.c | 29 -
 src/nwfilter/nwfilter_gentech_driver.c |  6 +++---
 5 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index a259062..7410047 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -42,6 +42,11 @@ struct _virNWFilterObj {
 virNWFilterDefPtr newDef;
 };
 
+struct _virNWFilterObjList {
+size_t count;
+virNWFilterObjPtr *objs;
+};
+
 
 static virNWFilterObjPtr
 virNWFilterObjNew(void)
@@ -115,6 +120,17 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters)
 }
 
 
+virNWFilterObjListPtr
+virNWFilterObjListNew(void)
+{
+virNWFilterObjListPtr nwfilters;
+
+if (VIR_ALLOC(nwfilters) < 0)
+return NULL;
+return nwfilters;
+}
+
+
 void
 virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters,
  virNWFilterObjPtr obj)
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index 4c19223..29d9086 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -28,11 +28,6 @@ typedef virNWFilterObj *virNWFilterObjPtr;
 
 typedef struct _virNWFilterObjList virNWFilterObjList;
 typedef virNWFilterObjList *virNWFilterObjListPtr;
-struct _virNWFilterObjList {
-size_t count;
-virNWFilterObjPtr *objs;
-};
-
 
 typedef struct _virNWFilterDriverState virNWFilterDriverState;
 typedef virNWFilterDriverState *virNWFilterDriverStatePtr;
@@ -40,7 +35,7 @@ struct _virNWFilterDriverState {
 virMutex lock;
 bool privileged;
 
-virNWFilterObjList nwfilters;
+virNWFilterObjListPtr nwfilters;
 
 char *configDir;
 bool watchingFirewallD;
@@ -55,6 +50,9 @@ virNWFilterObjGetNewDef(virNWFilterObjPtr obj);
 bool
 virNWFilterObjWantRemoved(virNWFilterObjPtr obj);
 
+virNWFilterObjListPtr
+virNWFilterObjListNew(void);
+
 void
 virNWFilterObjListFree(virNWFilterObjListPtr nwfilters);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 453d0fe..13f2ab9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -971,6 +971,7 @@ virNWFilterObjListFindByUUID;
 virNWFilterObjListFree;
 virNWFilterObjListGetNames;
 virNWFilterObjListLoadAllConfigs;
+virNWFilterObjListNew;
 virNWFilterObjListNumOfNWFilters;
 virNWFilterObjListRemove;
 virNWFilterObjLock;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 6516053..650c528 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -237,7 +237,10 @@ nwfilterStateInitialize(bool privileged,
 
 VIR_FREE(base);
 
-if (virNWFilterObjListLoadAllConfigs(>nwfilters, 
driver->configDir) < 0)
+if (!(driver->nwfilters = virNWFilterObjListNew()))
+goto error;
+
+if (virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir) 
< 0)
 goto error;
 
 nwfilterDriverUnlock();
@@ -289,7 +292,7 @@ nwfilterStateReload(void)
 virNWFilterWriteLockFilterUpdates();
 virNWFilterCallbackDriversLock();
 
-virNWFilterObjListLoadAllConfigs(>nwfilters, driver->configDir);
+virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir);
 
 virNWFilterCallbackDriversUnlock();
 virNWFilterUnlockFilterUpdates();
@@ -340,7 +343,7 @@ nwfilterStateCleanup(void)
 nwfilterDriverRemoveDBusMatches();
 
 /* free inactive nwfilters */
-virNWFilterObjListFree(>nwfilters);
+virNWFilterObjListFree(driver->nwfilters);
 
 VIR_FREE(driver->configDir);
 nwfilterDriverUnlock();
@@ -362,7 +365,7 @@ nwfilterLookupByUUID(virConnectPtr conn,
 virNWFilterPtr ret = NULL;
 
 nwfilterDriverLock();
-obj = virNWFilterObjListFindByUUID(>nwfilters, uuid);
+obj = virNWFilterObjListFindByUUID(driver->nwfilters, uuid);
 nwfilterDriverUnlock();
 
 if (!obj) {
@@ -393,7 +396,7 @@ nwfilterLookupByName(virConnectPtr conn,
 virNWFilterPtr ret = NULL;
 
 nwfilterDriverLock();
-obj = virNWFilterObjListFindByName(>nwfilters, name);
+obj = virNWFilterObjListFindByName(driver->nwfilters, name);
 nwfilterDriverUnlock();
 
 if (!obj) {
@@ -421,7 +424,7 @@ nwfilterConnectNumOfNWFilters(virConnectPtr conn)
 if (virConnectNumOfNWFiltersEnsureACL(conn) < 0)
 return -1;
 
-return virNWFilterObjListNumOfNWFilters(>nwfilters, conn,
+return virNWFilterObjListNumOfNWFilters(driver->nwfilters, conn,
 virConnectNumOfNWFiltersCheckACL);
 }
 
@@ -437,7 +440,7 @@ nwfilterConnectListNWFilters(virConnectPtr conn,
 return -1;
 
 nwfilterDriverLock();
-nnames = 

[libvirt] [PATCH 02/15] nwfilter: Use virNWFilterDefPtr rather than deref virNWFilterObjPtr

[John Ferlan]

Rather than dereferencing obj->def->XXX or nwfilters->objs[i]->X
create local virNWFilterObjPtr and virNWFilterDefPtr variables.

Future adjustments will be privatizing the object more, so this just
prepares the code for that reality.

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c  | 80 +++---
 src/nwfilter/nwfilter_driver.c | 33 ++---
 2 files changed, 72 insertions(+), 41 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index cb697f9..3c6bdbb 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -37,11 +37,16 @@ VIR_LOG_INIT("conf.virnwfilterobj");
 void
 virNWFilterObjFree(virNWFilterObjPtr obj)
 {
+virNWFilterDefPtr def;
+virNWFilterDefPtr newDef;
+
 if (!obj)
 return;
+def = obj->def;
+newDef = obj->newDef;
 
-virNWFilterDefFree(obj->def);
-virNWFilterDefFree(obj->newDef);
+virNWFilterDefFree(def);
+virNWFilterDefFree(newDef);
 
 virMutexDestroy(>lock);
 
@@ -87,12 +92,16 @@ virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters,
  const unsigned char *uuid)
 {
 size_t i;
+virNWFilterObjPtr obj;
+virNWFilterDefPtr def;
 
 for (i = 0; i < nwfilters->count; i++) {
-virNWFilterObjLock(nwfilters->objs[i]);
-if (!memcmp(nwfilters->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
-return nwfilters->objs[i];
-virNWFilterObjUnlock(nwfilters->objs[i]);
+obj = nwfilters->objs[i];
+virNWFilterObjLock(obj);
+def = obj->def;
+if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN))
+return obj;
+virNWFilterObjUnlock(obj);
 }
 
 return NULL;
@@ -104,12 +113,16 @@ virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters,
  const char *name)
 {
 size_t i;
+virNWFilterObjPtr obj;
+virNWFilterDefPtr def;
 
 for (i = 0; i < nwfilters->count; i++) {
-virNWFilterObjLock(nwfilters->objs[i]);
-if (STREQ_NULLABLE(nwfilters->objs[i]->def->name, name))
-return nwfilters->objs[i];
-virNWFilterObjUnlock(nwfilters->objs[i]);
+obj = nwfilters->objs[i];
+virNWFilterObjLock(obj);
+def = obj->def;
+if (STREQ_NULLABLE(def->name, name))
+return obj;
+virNWFilterObjUnlock(obj);
 }
 
 return NULL;
@@ -125,6 +138,7 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr 
nwfilters,
 size_t i;
 virNWFilterEntryPtr entry;
 virNWFilterObjPtr obj;
+virNWFilterDefPtr objdef;
 
 if (!def)
 return 0;
@@ -141,9 +155,9 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr 
nwfilters,
 obj = virNWFilterObjFindByName(nwfilters,
entry->include->filterref);
 if (obj) {
-rc = _virNWFilterObjDefLoopDetect(nwfilters,
-  obj->def, filtername);
-
+objdef = obj->def;
+rc = _virNWFilterObjDefLoopDetect(nwfilters, objdef,
+  filtername);
 virNWFilterObjUnlock(obj);
 if (rc < 0)
 break;
@@ -226,24 +240,26 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 virNWFilterDefPtr def)
 {
 virNWFilterObjPtr obj;
+virNWFilterDefPtr objdef;
 
-obj = virNWFilterObjFindByUUID(nwfilters, def->uuid);
+if ((obj = virNWFilterObjFindByUUID(nwfilters, def->uuid))) {
+objdef = obj->def;
 
-if (obj) {
-if (STRNEQ(def->name, obj->def->name)) {
+if (STRNEQ(def->name, objdef->name)) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("filter with same UUID but different name "
  "('%s') already exists"),
-   obj->def->name);
+   objdef->name);
 virNWFilterObjUnlock(obj);
 return NULL;
 }
 virNWFilterObjUnlock(obj);
 } else {
-obj = virNWFilterObjFindByName(nwfilters, def->name);
-if (obj) {
+if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(obj->def->uuid, uuidstr);
+
+objdef = obj->def;
+virUUIDFormat(objdef->uuid, uuidstr);
 virReportError(VIR_ERR_OPERATION_FAILED,
_("filter '%s' already exists with uuid %s"),
def->name, uuidstr);
@@ -261,8 +277,9 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 
 if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) {
 
-if (virNWFilterDefEqual(def, obj->def, false)) {
-virNWFilterDefFree(obj->def);
+objdef = obj->def;
+if 

[libvirt] [PATCH 00/15] Multiple cleanups within nwfilterobj and nwfilter drivers

[John Ferlan]

More adjustments in preparation for having virobject code handle the bulk
of the object management code.

I know it's a lot of patches, but reducing the amount of change overall has
the opposite effect of increasing the number patches to show the work flow.
Should make the "next" steps easier to review, but a couple of these will
be tedious.

John Ferlan (15):
  nwfilter: Use consistent naming for variables
  nwfilter: Use virNWFilterDefPtr rather than deref virNWFilterObjPtr
  nwfilter: Remove unused 'active' in virNWFilterObj
  nwfilter: Convert wantRemoved to bool
  nwfilter: Make _virNWFilterObjPtr private
  nwfilter: Introduce virNWFilterObjNew
  nwfilter: Rename some virNWFilterObj* API's
  nwfilter: Make _virNWFilterObjList private
  nwfilter: Make a common UUID lookup function from driver
  nwfilter: Replace virNWFilterConfigFile with virFileBuildPath
  nwfilter: Replace virNWFilterSaveDef with virNWFilterSaveConfig
  nwfilter: Move creation of configDir to driver initialization
  nwfilter: Move configFile name processing into driver
  nwfilter: Move save of config until after successful assign
  nwfilter: Move and rename virNWFilterSaveConfig

 src/conf/nwfilter_conf.c   | 107 +---
 src/conf/nwfilter_conf.h   |  16 +-
 src/conf/virnwfilterobj.c  | 308 ++---
 src/conf/virnwfilterobj.h  |  74 
 src/libvirt_private.syms   |  21 ++-
 src/nwfilter/nwfilter_driver.c | 148 ++--
 src/nwfilter/nwfilter_gentech_driver.c |  42 +++--
 7 files changed, 377 insertions(+), 339 deletions(-)

-- 
2.9.3

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

[libvirt] [PATCH 06/15] nwfilter: Introduce virNWFilterObjNew

[John Ferlan]

Perform the object initialization in a helper rather than inline

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c | 30 +-
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 9846751..dcfd1e7 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -43,6 +43,26 @@ struct _virNWFilterObj {
 };
 
 
+static virNWFilterObjPtr
+virNWFilterObjNew(void)
+{
+virNWFilterObjPtr obj;
+
+if (VIR_ALLOC(obj) < 0)
+return NULL;
+
+if (virMutexInitRecursive(>lock) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("cannot initialize mutex"));
+VIR_FREE(obj);
+return NULL;
+}
+
+virNWFilterObjLock(obj);
+return obj;
+}
+
+
 virNWFilterDefPtr
 virNWFilterObjGetDef(virNWFilterObjPtr obj)
 {
@@ -328,16 +348,8 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 return obj;
 }
 
-if (VIR_ALLOC(obj) < 0)
-return NULL;
-
-if (virMutexInitRecursive(>lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
-VIR_FREE(obj);
+if (!(obj = virNWFilterObjNew()))
 return NULL;
-}
-virNWFilterObjLock(obj);
 
 if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs,
 nwfilters->count, obj) < 0) {
-- 
2.9.3

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

[libvirt] [PATCH 01/15] nwfilter: Use consistent naming for variables

[John Ferlan]

When processing a virNWFilterPtr use 'nwfilter' as a variable name.

When processing a virNWFilterObjPtr use 'obj' as a variable name.

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c  | 92 +-
 src/conf/virnwfilterobj.h  |  7 ++--
 src/nwfilter/nwfilter_driver.c | 78 +--
 3 files changed, 89 insertions(+), 88 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 34d843c..cb697f9 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -62,15 +62,15 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters)
 
 void
 virNWFilterObjRemove(virNWFilterObjListPtr nwfilters,
- virNWFilterObjPtr nwfilter)
+ virNWFilterObjPtr obj)
 {
 size_t i;
 
-virNWFilterObjUnlock(nwfilter);
+virNWFilterObjUnlock(obj);
 
 for (i = 0; i < nwfilters->count; i++) {
 virNWFilterObjLock(nwfilters->objs[i]);
-if (nwfilters->objs[i] == nwfilter) {
+if (nwfilters->objs[i] == obj) {
 virNWFilterObjUnlock(nwfilters->objs[i]);
 virNWFilterObjFree(nwfilters->objs[i]);
 
@@ -174,16 +174,16 @@ virNWFilterObjDefLoopDetect(virNWFilterObjListPtr 
nwfilters,
 
 
 int
-virNWFilterObjTestUnassignDef(virNWFilterObjPtr nwfilter)
+virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj)
 {
 int rc = 0;
 
-nwfilter->wantRemoved = 1;
+obj->wantRemoved = 1;
 /* trigger the update on VMs referencing the filter */
 if (virNWFilterTriggerVMFilterRebuild())
 rc = -1;
 
-nwfilter->wantRemoved = 0;
+obj->wantRemoved = 0;
 
 return rc;
 }
@@ -225,29 +225,29 @@ virNWFilterObjPtr
 virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 virNWFilterDefPtr def)
 {
-virNWFilterObjPtr nwfilter;
+virNWFilterObjPtr obj;
 
-nwfilter = virNWFilterObjFindByUUID(nwfilters, def->uuid);
+obj = virNWFilterObjFindByUUID(nwfilters, def->uuid);
 
-if (nwfilter) {
-if (STRNEQ(def->name, nwfilter->def->name)) {
+if (obj) {
+if (STRNEQ(def->name, obj->def->name)) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("filter with same UUID but different name "
  "('%s') already exists"),
-   nwfilter->def->name);
-virNWFilterObjUnlock(nwfilter);
+   obj->def->name);
+virNWFilterObjUnlock(obj);
 return NULL;
 }
-virNWFilterObjUnlock(nwfilter);
+virNWFilterObjUnlock(obj);
 } else {
-nwfilter = virNWFilterObjFindByName(nwfilters, def->name);
-if (nwfilter) {
+obj = virNWFilterObjFindByName(nwfilters, def->name);
+if (obj) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(nwfilter->def->uuid, uuidstr);
+virUUIDFormat(obj->def->uuid, uuidstr);
 virReportError(VIR_ERR_OPERATION_FAILED,
_("filter '%s' already exists with uuid %s"),
def->name, uuidstr);
-virNWFilterObjUnlock(nwfilter);
+virNWFilterObjUnlock(obj);
 return NULL;
 }
 }
@@ -259,49 +259,49 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 }
 
 
-if ((nwfilter = virNWFilterObjFindByName(nwfilters, def->name))) {
+if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) {
 
-if (virNWFilterDefEqual(def, nwfilter->def, false)) {
-virNWFilterDefFree(nwfilter->def);
-nwfilter->def = def;
-return nwfilter;
+if (virNWFilterDefEqual(def, obj->def, false)) {
+virNWFilterDefFree(obj->def);
+obj->def = def;
+return obj;
 }
 
-nwfilter->newDef = def;
+obj->newDef = def;
 /* trigger the update on VMs referencing the filter */
 if (virNWFilterTriggerVMFilterRebuild()) {
-nwfilter->newDef = NULL;
-virNWFilterObjUnlock(nwfilter);
+obj->newDef = NULL;
+virNWFilterObjUnlock(obj);
 return NULL;
 }
 
-virNWFilterDefFree(nwfilter->def);
-nwfilter->def = def;
-nwfilter->newDef = NULL;
-return nwfilter;
+virNWFilterDefFree(obj->def);
+obj->def = def;
+obj->newDef = NULL;
+return obj;
 }
 
-if (VIR_ALLOC(nwfilter) < 0)
+if (VIR_ALLOC(obj) < 0)
 return NULL;
 
-if (virMutexInitRecursive(>lock) < 0) {
+if (virMutexInitRecursive(>lock) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot initialize mutex"));
-VIR_FREE(nwfilter);
+VIR_FREE(obj);
 return NULL;
 }
-virNWFilterObjLock(nwfilter);
-nwfilter->active = 0;
+virNWFilterObjLock(obj);
+

[libvirt] [PATCH 05/15] nwfilter: Make _virNWFilterObjPtr private

[John Ferlan]

Move the structure to virnwfilterobj.c and create necessary accessor API's
for the various fields.

Also make virNWFilterObjFree static since there's no external callers.

Signed-off-by: John Ferlan 
---
 src/conf/virnwfilterobj.c  | 32 +++-
 src/conf/virnwfilterobj.h  | 22 +-
 src/libvirt_private.syms   |  3 +++
 src/nwfilter/nwfilter_driver.c | 10 +-
 src/nwfilter/nwfilter_gentech_driver.c | 30 ++
 5 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 6e5c5b7..9846751 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -33,8 +33,38 @@
 
 VIR_LOG_INIT("conf.virnwfilterobj");
 
+struct _virNWFilterObj {
+virMutex lock;
 
-void
+bool wantRemoved;
+
+virNWFilterDefPtr def;
+virNWFilterDefPtr newDef;
+};
+
+
+virNWFilterDefPtr
+virNWFilterObjGetDef(virNWFilterObjPtr obj)
+{
+return obj->def;
+}
+
+
+virNWFilterDefPtr
+virNWFilterObjGetNewDef(virNWFilterObjPtr obj)
+{
+return obj->newDef;
+}
+
+
+bool
+virNWFilterObjWantRemoved(virNWFilterObjPtr obj)
+{
+return obj->wantRemoved;
+}
+
+
+static void
 virNWFilterObjFree(virNWFilterObjPtr obj)
 {
 virNWFilterDefPtr def;
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index f31938d..b02dcfa 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -26,16 +26,6 @@
 typedef struct _virNWFilterObj virNWFilterObj;
 typedef virNWFilterObj *virNWFilterObjPtr;
 
-struct _virNWFilterObj {
-virMutex lock;
-
-bool wantRemoved;
-
-virNWFilterDefPtr def;
-virNWFilterDefPtr newDef;
-};
-
-
 typedef struct _virNWFilterObjList virNWFilterObjList;
 typedef virNWFilterObjList *virNWFilterObjListPtr;
 struct _virNWFilterObjList {
@@ -56,6 +46,15 @@ struct _virNWFilterDriverState {
 bool watchingFirewallD;
 };
 
+virNWFilterDefPtr
+virNWFilterObjGetDef(virNWFilterObjPtr obj);
+
+virNWFilterDefPtr
+virNWFilterObjGetNewDef(virNWFilterObjPtr obj);
+
+bool
+virNWFilterObjWantRemoved(virNWFilterObjPtr obj);
+
 void
 virNWFilterObjListFree(virNWFilterObjListPtr nwfilters);
 
@@ -63,9 +62,6 @@ void
 virNWFilterObjRemove(virNWFilterObjListPtr nwfilters,
  virNWFilterObjPtr obj);
 
-void
-virNWFilterObjFree(virNWFilterObjPtr obj);
-
 virNWFilterObjPtr
 virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters,
  const unsigned char *uuid);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 83e979a..dd6cb98 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -965,7 +965,9 @@ virNodeDeviceObjUnlock;
 virNWFilterObjAssignDef;
 virNWFilterObjFindByName;
 virNWFilterObjFindByUUID;
+virNWFilterObjGetDef;
 virNWFilterObjGetNames;
+virNWFilterObjGetNewDef;
 virNWFilterObjListExport;
 virNWFilterObjListFree;
 virNWFilterObjLoadAllConfigs;
@@ -974,6 +976,7 @@ virNWFilterObjNumOfNWFilters;
 virNWFilterObjRemove;
 virNWFilterObjTestUnassignDef;
 virNWFilterObjUnlock;
+virNWFilterObjWantRemoved;
 
 
 # conf/virsecretobj.h
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 7e88a40..dd3645a 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -370,7 +370,7 @@ nwfilterLookupByUUID(virConnectPtr conn,
"%s", _("no nwfilter with matching uuid"));
 goto cleanup;
 }
-def = obj->def;
+def = virNWFilterObjGetDef(obj);
 
 if (virNWFilterLookupByUUIDEnsureACL(conn, def) < 0)
 goto cleanup;
@@ -401,7 +401,7 @@ nwfilterLookupByName(virConnectPtr conn,
_("no nwfilter with matching name '%s'"), name);
 goto cleanup;
 }
-def = obj->def;
+def = virNWFilterObjGetDef(obj);
 
 if (virNWFilterLookupByNameEnsureACL(conn, def) < 0)
 goto cleanup;
@@ -493,7 +493,7 @@ nwfilterDefineXML(virConnectPtr conn,
 if (!(obj = virNWFilterObjAssignDef(>nwfilters, def)))
 goto cleanup;
 def = NULL;
-objdef = obj->def;
+objdef = virNWFilterObjGetDef(obj);
 
 if (virNWFilterSaveDef(driver->configDir, objdef) < 0) {
 virNWFilterObjRemove(>nwfilters, obj);
@@ -531,7 +531,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter)
"%s", _("no nwfilter with matching uuid"));
 goto cleanup;
 }
-def = obj->def;
+def = virNWFilterObjGetDef(obj);
 
 if (virNWFilterUndefineEnsureACL(nwfilter->conn, def) < 0)
 goto cleanup;
@@ -580,7 +580,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter,
"%s", _("no nwfilter with matching uuid"));
 goto cleanup;
 }
-def = obj->def;
+def = virNWFilterObjGetDef(obj);
 
 if (virNWFilterGetXMLDescEnsureACL(nwfilter->conn, def) < 0)
 goto cleanup;
diff --git a/src/nwfilter/nwfilter_gentech_driver.c 

[libvirt] [PATCH go-xml] remove superfluous state & omitempty entries

[Ryan Goodfellow]

This commit removes the superfluous state and omit entries from my last
commit. It factors the 'state' data member into a DomainFeatureState
struct and only uses that struct where necessary e.g. HyperV features
as well as a few top level features such as PMU, HAP etc. The HyperV
suffix has also been added to features that are specific to
HyperV.
---
 domain.go  | 62 +-
 domain_test.go | 26 +---
 2 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/domain.go b/domain.go
index c9ffaef..b4b2256 100644
--- a/domain.go
+++ b/domain.go
@@ -372,61 +372,61 @@ type DomainCPU struct {
 }
 
 type DomainFeature struct {
+}
+
+type DomainFeatureState struct {
State string `xml:"state,attr,omitempty"`
 }
 
 type DomainFeatureAPIC struct {
-   DomainFeature
EOI string `xml:"eio,attr,omitempty"`
 }
 
-type DomainFeatureVendorId struct {
-   DomainFeature
+type DomainFeatureHyperVVendorId struct {
+   DomainFeatureState
Value string `xml:"value,attr,omitempty"`
 }
 
-type DomainFeatureSpinlocks struct {
-   DomainFeature
+type DomainFeatureHyperVSpinlocks struct {
+   DomainFeatureState
Retries uint `xml:"retries,attr,omitempty"`
 }
 
 type DomainFeatureHyperV struct {
DomainFeature
-   Relaxed   *DomainFeature  `xml:"relaxed,omitempty"`
-   VAPIC *DomainFeature  `xml:"vapic,omitempty"`
-   Spinlocks *DomainFeatureSpinlocks `xml:"spinlocks,omitempty"`
-   VPIndex   *DomainFeature  `xml:"vpindex,omitempty"`
-   Runtime   *DomainFeature  `xml:"runtime,omitempty"`
-   Synic *DomainFeature  `xml:"synic,omitempty"`
-   STimer*DomainFeature  `xml:"stimer,omitempty"`
-   Reset *DomainFeature  `xml:"reset,omitempty"`
-   VendorId  *DomainFeatureVendorId  `xml:"vendor_id,omitempty"`
+   Relaxed   *DomainFeatureState   `xml:"relaxed"`
+   VAPIC *DomainFeatureState   `xml:"vapic"`
+   Spinlocks *DomainFeatureHyperVSpinlocks `xml:"spinlocks"`
+   VPIndex   *DomainFeatureState   `xml:"vpindex"`
+   Runtime   *DomainFeatureState   `xml:"runtime"`
+   Synic *DomainFeatureState   `xml:"synic"`
+   STimer*DomainFeatureState   `xml:"stimer"`
+   Reset *DomainFeatureState   `xml:"reset"`
+   VendorId  *DomainFeatureHyperVVendorId  `xml:"vendor_id"`
 }
 
 type DomainFeatureKVM struct {
-   DomainFeature
-   Hidden *DomainFeature `xml:"hidden,omitempty"`
+   Hidden *DomainFeatureState `xml:"hidden"`
 }
 
 type DomainFeatureGIC struct {
-   DomainFeature
Version string `xml:"version,attr,omitempty"`
 }
 
 type DomainFeatureList struct {
-   PAE*DomainFeature   `xml:"pae,omitempty"`
-   ACPI   *DomainFeature   `xml:"acpi,omitempty"`
-   APIC   *DomainFeatureAPIC   `xml:"apic,omitempty"`
-   HAP*DomainFeature   `xml:"hap,omitempty"`
-   Viridian   *DomainFeature   `xml:"viridian,omitempty"`
-   PrivNet*DomainFeature   `xml:"privnet,omitempty"`
-   HyperV *DomainFeatureHyperV `xml:"hyperv,omitempty"`
-   KVM*DomainFeatureKVM`xml:"kvm,omitempty"`
-   PVSpinlock *DomainFeature   `xml:"pvspinlock,omitempty"`
-   PMU*DomainFeature   `xml:"pmu,omitempty"`
-   VMPort *DomainFeature   `xml:"vmport,omitempty"`
-   GIC*DomainFeatureGIC`xml:"gic,omitempty"`
-   SMM*DomainFeature   `xml:"smm,omitempty"`
+   PAE*DomainFeature   `xml:"pae"`
+   ACPI   *DomainFeature   `xml:"acpi"`
+   APIC   *DomainFeatureAPIC   `xml:"apic"`
+   HAP*DomainFeatureState  `xml:"hap"`
+   Viridian   *DomainFeature   `xml:"viridian"`
+   PrivNet*DomainFeature   `xml:"privnet"`
+   HyperV *DomainFeatureHyperV `xml:"hyperv"`
+   KVM*DomainFeatureKVM`xml:"kvm"`
+   PVSpinlock *DomainFeatureState  `xml:"pvspinlock"`
+   PMU*DomainFeatureState  `xml:"pmu"`
+   VMPort *DomainFeatureState  `xml:"vmport"`
+   GIC*DomainFeatureGIC`xml:"gic"`
+   SMM*DomainFeatureState  `xml:"smm"`
 }
 
 type Domain struct {
@@ -446,7 +446,7 @@ type Domain struct {
OnPoweroffstring `xml:"on_poweroff,omitempty"`
OnReboot  string `xml:"on_reboot,omitempty"`
OnCrash   string `xml:"on_crash,omitempty"`
-   Features  *DomainFeatureList `xml:"features,omitempty"`
+   Features  *DomainFeatureList `xml:"features"`
 }
 
 func (d *Domain) Unmarshal(doc string) error {
diff --git a/domain_test.go b/domain_test.go
index e25007e..11156c1 100644
--- a/domain_test.go
+++ b/domain_test.go
@@ -753,22 +753,26 @@ var domainTestData = []struct {
 

[libvirt] [PATCH v3 3/5] hyperv: add hypervInvokeMethod

[Sri Ramanujam]

This commit adds support for invoking methods on remote objects
via hypervInvokeMethod.
---
 src/hyperv/hyperv_wmi.c | 569 
 src/hyperv/hyperv_wmi.h |   3 +
 src/hyperv/openwsman.h  |   4 +
 3 files changed, 576 insertions(+)

diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 1960c4c..deea907 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -24,6 +24,7 @@
  */
 
 #include 
+#include 
 
 #include "internal.h"
 #include "virerror.h"
@@ -34,11 +35,14 @@
 #include "hyperv_private.h"
 #include "hyperv_wmi.h"
 #include "virstring.h"
+#include "openwsman.h"
+#include "virlog.h"
 
 #define WS_SERIALIZER_FREE_MEM_WORKS 0
 
 #define VIR_FROM_THIS VIR_FROM_HYPERV
 
+VIR_LOG_INIT("hyperv.hyperv_wmi");
 
 static int
 hypervGetWmiClassInfo(hypervPrivate *priv, hypervWmiClassInfoListPtr list,
@@ -406,6 +410,571 @@ hypervAddEmbeddedParam(hypervInvokeParamsListPtr params, 
hypervPrivate *priv,
 }
 
 
+/*
+ * Serializing parameters to XML and invoking methods
+ */
+
+static int
+hypervGetCimTypeInfo(hypervCimTypePtr typemap, const char *name,
+hypervCimTypePtr *property)
+{
+size_t i = 0;
+while (typemap[i].name[0] != '\0') {
+if (STREQ(typemap[i].name, name)) {
+*property = [i];
+return 0;
+}
+i++;
+}
+
+return -1;
+}
+
+
+static int
+hypervCreateInvokeXmlDoc(hypervInvokeParamsListPtr params, WsXmlDocH *docRoot)
+{
+int result = -1;
+char *method = NULL;
+WsXmlNodeH xmlNodeMethod = NULL;
+
+if (virAsprintf(, "%s_INPUT", params->method) < 0)
+goto error;
+
+*docRoot = ws_xml_create_doc(NULL, method);
+if (*docRoot == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Could not instantiate XML document"));
+goto error;
+}
+
+xmlNodeMethod = xml_parser_get_root(*docRoot);
+if (xmlNodeMethod == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Could not get root node of XML document"));
+goto error;
+}
+
+/* add resource URI as namespace */
+ws_xml_set_ns(xmlNodeMethod, params->resourceUri, "p");
+
+result = 0;
+goto cleanup;
+
+ error:
+if (*docRoot != NULL) {
+ws_xml_destroy_doc(*docRoot);
+*docRoot = NULL;
+}
+ cleanup:
+VIR_FREE(method);
+return result;
+}
+
+static int
+hypervSerializeSimpleParam(hypervParamPtr p, const char *resourceUri,
+WsXmlNodeH *methodNode)
+{
+int result = -1;
+WsXmlNodeH xmlNodeParam = NULL;
+
+xmlNodeParam = ws_xml_add_child(*methodNode, resourceUri,
+p->simple.name, p->simple.value);
+if (xmlNodeParam == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Could not create simple param"));
+goto cleanup;
+}
+
+result = 0;
+
+ cleanup:
+return result;
+}
+
+static int
+hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv,
+const char *resourceUri, WsXmlDocH doc, WsXmlNodeH *methodNode)
+{
+int result = -1;
+WsXmlNodeH xmlNodeParam = NULL,
+   xmlNodeTemp = NULL,
+   xmlNodeAddr = NULL,
+   xmlNodeRef = NULL;
+xmlNodePtr xmlNodeAddrPtr = NULL,
+   xmlNodeRefPtr = NULL;
+WsXmlDocH xmlDocResponse = NULL;
+xmlDocPtr docPtr = (xmlDocPtr) doc->parserDoc;
+WsXmlNsH ns = NULL;
+client_opt_t *options = NULL;
+filter_t *filter = NULL;
+char *enumContext = NULL;
+char *query_string = NULL;
+
+/* init and set up options */
+options = wsmc_options_init();
+if (!options) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init 
options"));
+goto cleanup;
+}
+wsmc_set_action_option(options, FLAG_ENUMERATION_ENUM_EPR);
+
+/* Get query and create filter based on it */
+query_string = virBufferContentAndReset(p->epr.query);
+filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string);
+if (!filter) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create WQL 
filter"));
+goto cleanup;
+}
+
+/* enumerate based on the filter from this query */
+xmlDocResponse = wsmc_action_enumerate(priv->client, p->epr.info->rootUri,
+options, filter);
+if (hypervVerifyResponse(priv->client, xmlDocResponse, "enumeration") < 0)
+goto cleanup;
+
+/* Get context */
+enumContext = wsmc_get_enum_context(xmlDocResponse);
+ws_xml_destroy_doc(xmlDocResponse);
+
+/* Pull using filter and enum context */
+xmlDocResponse = wsmc_action_pull(priv->client, resourceUri, options,
+filter, enumContext);
+
+if (hypervVerifyResponse(priv->client, xmlDocResponse, "pull") < 0)
+goto cleanup;
+
+/* drill down and extract EPR node children */
+if (!(xmlNodeTemp = ws_xml_get_soap_body(xmlDocResponse))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", 

[libvirt] [PATCH v3 5/5] hyperv: Add support for virDomainSetMemory

[Sri Ramanujam]

Introduces support for virDomainSetMemory. This also serves an an
example for how to use the new method invocation API with a more
complicated method, this time including an EPR and embedded param.
---
 src/hyperv/hyperv_driver.c| 96 +++
 src/hyperv/hyperv_wmi.c   | 51 +++
 src/hyperv/hyperv_wmi.h   | 11 
 src/hyperv/hyperv_wmi_generator.input | 30 +++
 4 files changed, 188 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 9562d5a..104e13f 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1459,6 +1459,100 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int 
codeset,
 }
 
 
+static int
+hypervDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory,
+unsigned int flags)
+{
+int result = -1;
+char uuid_string[VIR_UUID_STRING_BUFLEN];
+hypervPrivate *priv = domain->conn->privateData;
+char *memory_str = NULL;
+hypervInvokeParamsListPtr params = NULL;
+unsigned long memory_mb = memory / 1024;
+Msvm_VirtualSystemSettingData *vssd = NULL;
+Msvm_MemorySettingData *memsd = NULL;
+virBuffer eprQuery = VIR_BUFFER_INITIALIZER;
+virHashTablePtr memResource = NULL;
+
+virCheckFlags(0, -1);
+
+/* memory has to be a multiple of 2; round up if necessary */
+if (memory_mb % 2) memory_mb++;
+
+if (virAsprintf(_str, "%lu", memory_mb) < 0)
+goto cleanup;
+
+virUUIDFormat(domain->uuid, uuid_string);
+
+if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, 
) < 0)
+goto cleanup;
+
+if (hypervGetMsvmMemorySettingDataFromVSSD(priv, 
vssd->data.common->InstanceID,
+) < 0)
+goto cleanup;
+
+if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
+params = hypervInitInvokeParamsList(priv, 
"ModifyVirtualSystemResources",
+MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR,
+Msvm_VirtualSystemManagementService_WmiInfo);
+
+virBufferAddLit(, MSVM_COMPUTERSYSTEM_WQL_SELECT);
+virBufferAsprintf(, "where Name = \"%s\"", uuid_string);
+
+if (hypervAddEprParam(params, "ComputerSystem", priv, ,
+Msvm_ComputerSystem_WmiInfo) < 0)
+goto cleanup;
+
+} else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) {
+params = hypervInitInvokeParamsList(priv, "ModifyResourceSettings",
+MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR,
+Msvm_VirtualSystemManagementService_WmiInfo);
+}
+
+memResource = hypervCreateEmbeddedParam(priv, 
Msvm_MemorySettingData_WmiInfo);
+if (memResource == NULL)
+goto cleanup;
+
+if (hypervSetEmbeddedProperty(memResource, "VirtualQuantity", memory_str) 
< 0)
+goto cleanup;
+
+if (hypervSetEmbeddedProperty(memResource, "InstanceID",
+memsd->data.common->InstanceID) < 0)
+goto cleanup;
+
+
+if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
+if (hypervAddEmbeddedParam(params, priv, "ResourceSettingData",
+memResource, Msvm_MemorySettingData_WmiInfo) < 0)
+goto cleanup;
+
+} else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) {
+if (hypervAddEmbeddedParam(params, priv, "ResourceSettings",
+memResource, Msvm_MemorySettingData_WmiInfo) < 0)
+goto cleanup;
+}
+
+if (hypervInvokeMethod(priv, params, NULL) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set 
memory"));
+goto cleanup;
+}
+
+result = 0;
+ cleanup:
+if (memory_str) VIR_FREE(memory_str);
+hypervFreeObject(priv, (hypervObject *) vssd);
+hypervFreeObject(priv, (hypervObject *) memsd);
+return result;
+}
+
+
+static int
+hypervDomainSetMemory(virDomainPtr domain, unsigned long memory)
+{
+return hypervDomainSetMemoryFlags(domain, memory, 0);
+}
+
+
 static virHypervisorDriver hypervHypervisorDriver = {
 .name = "Hyper-V",
 .connectOpen = hypervConnectOpen, /* 0.9.5 */
@@ -1493,6 +1587,8 @@ static virHypervisorDriver hypervHypervisorDriver = {
 .domainHasManagedSaveImage = hypervDomainHasManagedSaveImage, /* 0.9.5 */
 .domainManagedSaveRemove = hypervDomainManagedSaveRemove, /* 0.9.5 */
 .domainSendKey = hypervDomainSendKey, /* TODO: version */
+.domainSetMemory = hypervDomainSetMemory, /* TODO: version */
+.domainSetMemoryFlags = hypervDomainSetMemoryFlags, /* TODO: version */
 .connectIsAlive = hypervConnectIsAlive, /* 0.9.8 */
 };
 
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index df248e0..756326d 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -1616,3 +1616,54 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain,
 
 return 0;
 }
+
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Msvm_VirtualSystemSettingData
+ */
+

[libvirt] [PATCH v3 4/5] hyperv: support virDomainSendKey

[Sri Ramanujam]

This commit adds support for virDomainSendKey. It also serves as an
example of how to use the new method invocation APIs with a single
"simple" type parameter.
---
 src/hyperv/hyperv_driver.c| 85 ++
 src/hyperv/hyperv_wmi.c   |  7 +++
 src/hyperv/hyperv_wmi.h   |  3 +-
 src/hyperv/hyperv_wmi_generator.input | 86 +++
 4 files changed, 180 insertions(+), 1 deletion(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 0ca5971..9562d5a 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -35,6 +35,7 @@
 #include "hyperv_wmi.h"
 #include "openwsman.h"
 #include "virstring.h"
+#include "virkeycode.h"
 
 #define VIR_FROM_THIS VIR_FROM_HYPERV
 
@@ -1373,6 +1374,89 @@ hypervConnectListAllDomains(virConnectPtr conn,
 #undef MATCH
 
 
+static int
+hypervDomainSendKey(virDomainPtr domain, unsigned int codeset,
+unsigned int holdtime ATTRIBUTE_UNUSED, unsigned int *keycodes,
+int nkeycodes, unsigned int flags)
+{
+int result = -1;
+size_t i = 0;
+int keycode = 0;
+int *translatedKeycodes = NULL;
+hypervPrivate *priv = domain->conn->privateData;
+char uuid_string[VIR_UUID_STRING_BUFLEN];
+char *selector = NULL;
+Msvm_ComputerSystem *computerSystem = NULL;
+Msvm_Keyboard *keyboard = NULL;
+virBuffer query = VIR_BUFFER_INITIALIZER;
+hypervInvokeParamsListPtr params = NULL;
+
+virCheckFlags(0, -1);
+
+virUUIDFormat(domain->uuid, uuid_string);
+
+if (hypervMsvmComputerSystemFromDomain(domain, ) < 0)
+goto cleanup;
+
+virBufferAsprintf(,
+"associators of "
+"{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
+"Name=\"%s\"} "
+"where ResultClass = Msvm_Keyboard",
+uuid_string);
+
+
+if (hypervGetMsvmKeyboardList(priv, , ) < 0)
+goto cleanup;
+
+/* translate keycodes to xt and generate keyup scancodes. */
+translatedKeycodes = (int *) keycodes;
+for (i = 0; i < nkeycodes; i++) {
+if (codeset != VIR_KEYCODE_SET_WIN32) {
+keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_WIN32,
+translatedKeycodes[i]);
+
+if (keycode < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Could not translate keycode"));
+goto cleanup;
+}
+translatedKeycodes[i] = keycode;
+}
+}
+
+if (virAsprintf(,
+"CreationClassName=Msvm_Keyboard=%s&"
+"SystemCreationClassName=Msvm_ComputerSystem&"
+"SystemName=%s", keyboard->data.common->DeviceID, uuid_string) 
< 0)
+goto cleanup;
+
+/* type the keys */
+for (i = 0; i < nkeycodes; i++) {
+char keycodeStr[sizeof(int) * 3 + 2];
+snprintf(keycodeStr, sizeof(keycodeStr), "%d", translatedKeycodes[i]);
+
+/* params obj takes ownership of selector */
+params = hypervInitInvokeParamsList(priv, "TypeKey", selector,
+Msvm_Keyboard_WmiInfo);
+if (hypervAddSimpleParam(params, "keyCode", keycodeStr) < 0)
+goto cleanup;
+
+if (hypervInvokeMethod(priv, params, NULL) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not press key %d"),
+   translatedKeycodes[i]);
+goto cleanup;
+}
+}
+
+result = 0;
+
+ cleanup:
+hypervFreeObject(priv, (hypervObject *) keyboard);
+hypervFreeObject(priv, (hypervObject *) computerSystem);
+virBufferFreeAndReset();
+return result;
+}
 
 
 static virHypervisorDriver hypervHypervisorDriver = {
@@ -1408,6 +1492,7 @@ static virHypervisorDriver hypervHypervisorDriver = {
 .domainManagedSave = hypervDomainManagedSave, /* 0.9.5 */
 .domainHasManagedSaveImage = hypervDomainHasManagedSaveImage, /* 0.9.5 */
 .domainManagedSaveRemove = hypervDomainManagedSaveRemove, /* 0.9.5 */
+.domainSendKey = hypervDomainSendKey, /* TODO: version */
 .connectIsAlive = hypervConnectIsAlive, /* 0.9.8 */
 };
 
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index deea907..df248e0 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -1323,6 +1323,13 @@ hypervGetMsvmMemorySettingDataList(hypervPrivate *priv, 
virBufferPtr query,
  (hypervObject **) list);
 }
 
+int hypervGetMsvmKeyboardList(hypervPrivate *priv, virBufferPtr query,
+  Msvm_Keyboard **list)
+{
+return hypervGetWmiClassList(priv, Msvm_Keyboard_WmiInfo, query,
+ (hypervObject **) list);
+}
+
 
 
 /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h
index 4196f51..740bdcb 100644
--- a/src/hyperv/hyperv_wmi.h
+++ 

[libvirt] [PATCH v3 0/5] Hyper-V method invocation

[Sri Ramanujam]

Changes from v2:

* Correctly manage and free invocation parameters
* Fixed a couple of other memory leaks found while fixing the above issue
* Minor code changes from review

I also forgot to mention previously that we have test servers available for
reviewers as we introduce new functionality to the driver. Let me know if you
want access to them :)

Sri Ramanujam (5):
  hyperv: Functions to work with invocation parameters.
  hyperv: Generate object property type information.
  hyperv: add hypervInvokeMethod
  hyperv: support virDomainSendKey
  hyperv: Add support for virDomainSetMemory

 src/hyperv/hyperv_driver.c| 181 +++
 src/hyperv/hyperv_wmi.c   | 891 ++
 src/hyperv/hyperv_wmi.h   |  93 +++-
 src/hyperv/hyperv_wmi_classes.h   |  19 +
 src/hyperv/hyperv_wmi_generator.input | 116 +
 src/hyperv/hyperv_wmi_generator.py|  15 +-
 src/hyperv/openwsman.h|   4 +
 7 files changed, 1317 insertions(+), 2 deletions(-)

-- 
2.9.3

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

[libvirt] [PATCH v3 1/5] hyperv: Functions to work with invocation parameters.

[Sri Ramanujam]

This commit introduces functionality for creating and working with
invoke parameters. This commit does not include any code for serializing
and actually performing the method invocations; it merely defines the
functions and API for using invocation parameters in driver code.

HYPERV_DEFAULT_PARAM_COUNT was chosen because almost no method
invocations have more than 4 parameters.

Functions added:
* hypervInitInvokeParamsList
* hypervFreeInvokeParams
* hypervAddSimpleParam
* hypervAddEprParam
* hypervCreateEmbeddedParam
* hypervSetEmbeddedProperty
* hypervAddEmbeddedParam
---
 src/hyperv/hyperv_wmi.c | 264 
 src/hyperv/hyperv_wmi.h |  78 +-
 2 files changed, 341 insertions(+), 1 deletion(-)

diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index a3c7dc0..1960c4c 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -2,6 +2,7 @@
  * hyperv_wmi.c: general WMI over WSMAN related functions and structures for
  *   managing Microsoft Hyper-V hosts
  *
+ * Copyright (C) 2017 Datto Inc
  * Copyright (C) 2014 Red Hat, Inc.
  * Copyright (C) 2011 Matthias Bolte 
  * Copyright (C) 2009 Michael Sievers 
@@ -142,6 +143,269 @@ hypervVerifyResponse(WsManClient *client, WsXmlDocH 
response,
 }
 
 
+/*
+ * Methods to work with method invocation parameters
+ */
+
+/*
+ * hypervInitInvokeParamsList:
+ * @priv: hypervPrivate object associated with the connection.
+ * @method: The name of the method you are calling
+ * @selector: The selector for the object you are invoking the method on
+ * @obj: The WmiInfo of the object class you are invoking the method on.
+ *
+ * Create a new InvokeParamsList object for the method call.
+ *
+ * Returns a pointer to the newly instantiated object on success, which should
+ * be freed by hypervInvokeMethod. Otherwise returns NULL.
+ */
+hypervInvokeParamsListPtr
+hypervInitInvokeParamsList(hypervPrivate *priv, const char *method,
+const char *selector, hypervWmiClassInfoListPtr obj)
+{
+hypervInvokeParamsListPtr params = NULL;
+hypervWmiClassInfoPtr info = NULL;
+
+if (hypervGetWmiClassInfo(priv, obj, ) < 0)
+goto cleanup;
+
+if (VIR_ALLOC(params) < 0)
+goto cleanup;
+
+if (VIR_ALLOC_N(params->params,
+HYPERV_DEFAULT_PARAM_COUNT) < 0) {
+VIR_FREE(params);
+goto cleanup;
+}
+
+params->method = method;
+params->ns = info->rootUri;
+params->resourceUri = info->resourceUri;
+params->selector = selector;
+params->nbParams = 0;
+params->nbAvailParams = HYPERV_DEFAULT_PARAM_COUNT;
+
+ cleanup:
+return params;
+}
+
+/*
+ * hypervFreeInvokeParams:
+ * @params: Params object to be freed
+ *
+ */
+void
+hypervFreeInvokeParams(hypervInvokeParamsListPtr params)
+{
+hypervParamPtr p = NULL;
+int i = 0;
+
+if (params == NULL)
+return;
+
+for (i = 0; i < params->nbParams; i++) {
+p = &(params->params[i]);
+
+switch(p->type) {
+case HYPERV_SIMPLE_PARAM:
+VIR_FREE(p->simple.name);
+VIR_FREE(p->simple.value);
+break;
+case HYPERV_EPR_PARAM:
+virBufferFreeAndReset(p->epr.query);
+break;
+case HYPERV_EMBEDDED_PARAM:
+virHashFree(p->embedded.table);
+break;
+}
+}
+
+VIR_DISPOSE_N(params->params, params->nbAvailParams);
+VIR_FREE(params);
+}
+
+static inline int
+hypervCheckParams(hypervInvokeParamsListPtr params)
+{
+if (params->nbParams + 1 > params->nbAvailParams) {
+if (VIR_EXPAND_N(params->params, params->nbAvailParams, 5) < 0)
+return -1;
+}
+
+return 0;
+}
+
+/*
+ * hypervAddSimpleParam:
+ * @params: Params object to add to
+ * @name: Name of the parameter
+ * @value: Value of the parameter
+ *
+ * Add a param of type HYPERV_SIMPLE_PARAM, which is essentially a serialized
+ * key/value pair.
+ *
+ * Returns -1 on failure, 0 on success.
+ */
+int
+hypervAddSimpleParam(hypervInvokeParamsListPtr params, const char *name,
+const char *value)
+{
+int result = -1;
+hypervParamPtr p = NULL;
+
+if (hypervCheckParams(params) < 0)
+goto cleanup;
+
+p = >params[params->nbParams];
+p->type = HYPERV_SIMPLE_PARAM;
+
+if (VIR_STRDUP(p->simple.name, name) < 0)
+goto cleanup;
+
+if (VIR_STRDUP(p->simple.value, value) < 0) {
+VIR_FREE(p->simple.name);
+goto cleanup;
+}
+params->nbParams++;
+
+result = 0;
+
+ cleanup:
+return result;
+}
+
+/*
+ * hypervAddEprParam:
+ * @params: Params object to add to
+ * @name: Parameter name
+ * @priv: hypervPrivate object associated with the connection
+ * @query: WQL filter
+ * @eprInfo: WmiInfo of the object being filtered
+ *
+ * Adds an EPR param to the params list. Returns -1 on failure, 0 on success.
+ 

[libvirt] [PATCH v3 2/5] hyperv: Generate object property type information.

[Sri Ramanujam]

Update the generator to generate basic property type information for
each CIM object representation. Right now, it generates arrays of
hypervCimType structs:

struct _hypervCimType {
const char *name;
const char *type;
bool isArray;
};
---
 src/hyperv/hyperv_wmi_classes.h| 19 +++
 src/hyperv/hyperv_wmi_generator.py | 15 ++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/hyperv/hyperv_wmi_classes.h b/src/hyperv/hyperv_wmi_classes.h
index f7d596f..ce4643e 100644
--- a/src/hyperv/hyperv_wmi_classes.h
+++ b/src/hyperv/hyperv_wmi_classes.h
@@ -1,6 +1,7 @@
 /*
  * hyperv_wmi_classes.h: WMI classes for managing Microsoft Hyper-V hosts
  *
+ * Copyright (C) 2017 Datto Inc
  * Copyright (C) 2011 Matthias Bolte 
  * Copyright (C) 2009 Michael Sievers 
  *
@@ -23,6 +24,7 @@
 #ifndef __HYPERV_WMI_CLASSES_H__
 # define __HYPERV_WMI_CLASSES_H__
 
+# include "internal.h"
 # include "openwsman.h"
 
 # include "hyperv_wmi_classes.generated.typedef"
@@ -96,6 +98,21 @@ enum _Msvm_ConcreteJob_JobState {
 };
 
 
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * WMI
+ */
+
+typedef struct _hypervCimType hypervCimType;
+typedef hypervCimType *hypervCimTypePtr;
+struct _hypervCimType {
+/* Parameter name */
+const char *name;
+/* Parameter type */
+const char *type;
+/* whether parameter is an array type */
+bool isArray;
+};
+
 typedef struct _hypervWmiClassInfo hypervWmiClassInfo;
 typedef hypervWmiClassInfo *hypervWmiClassInfoPtr;
 struct _hypervWmiClassInfo {
@@ -109,6 +126,8 @@ struct _hypervWmiClassInfo {
 const char *resourceUri;
 /* The wsman serializer info - one of the *_TypeInfo structs */
 XmlSerializerInfo *serializerInfo;
+/* Property type information */
+hypervCimTypePtr propertyInfo;
 };
 
 
diff --git a/src/hyperv/hyperv_wmi_generator.py 
b/src/hyperv/hyperv_wmi_generator.py
index 9aee0b9..9c0acce 100755
--- a/src/hyperv/hyperv_wmi_generator.py
+++ b/src/hyperv/hyperv_wmi_generator.py
@@ -122,6 +122,14 @@ class WmiClass:
 
 source += "SER_END_ITEMS(%s_Data);\n\n" % cls.name
 
+# also generate typemap data while we're here
+source += "hypervCimType %s_Typemap[] = {\n" % cls.name
+
+for property in cls.properties:
+source += property.generate_typemap()
+source += '{ "", "", 0 },\n' # null terminated
+source += '};\n\n'
+
 
 source += self._define_WmiInfo_struct()
 source += "\n\n"
@@ -222,7 +230,8 @@ class WmiClass:
 source += ".version = NULL,\n"
 source += ".rootUri = %s,\n" % cls.uri_info.rootUri
 source += ".resourceUri = %s_RESOURCE_URI,\n" % 
cls.name.upper()
-source += ".serializerInfo = %s_Data_TypeInfo\n" % 
cls.name
+source += ".serializerInfo = %s_Data_TypeInfo,\n" % 
cls.name
+source += ".propertyInfo = %s_Typemap\n" % cls.name
 source += "},\n"
 
 source += "}\n"
@@ -374,6 +383,10 @@ class Property:
% (Property.typemap[self.type], class_name.upper(), 
self.name)
 
 
+def generate_typemap(self):
+return '{ "%s", "%s", %s },\n' % (self.name, self.type.lower(), 
str(self.is_array).lower())
+
+
 
 def open_and_print(filename):
 if filename.startswith("./"):
-- 
2.9.3

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

[libvirt] [PATCH] qemu: Do not overwrite existing messages in hotplug

[Eric Farman]

I tried to attach a SCSI LUN to two different guests, and forgot
to specify "shareable" in the hostdev XML.  Attaching the device
to the second guest failed, but the message was not helpful in
telling me what I was doing wrong:

  $ cat scsi_scratch_disk.xml

  


  


  $ virsh attach-device dasd_sles_d99c scsi_scratch_disk.xml
  Device attached successfully

  $ virsh attach-device dasd_fedora_0e1e scsi_scratch_disk.xml
  error: Failed to attach device from scsi_scratch_disk.xml
  error: internal error: Unable to prepare scsi hostdev: 
scsi_host3:0:15:1074151456

I eventually discovered my error, but thought it was weird that
Libvirt doesn't provide something more helpful in this case.
Looking over the code we had just gone through, I commented out
the "internal error" message, and got something more useful:

  $ virsh attach-device dasd_fedora_0e1e scsi_scratch_disk.xml
  error: Failed to attach device from scsi_scratch_disk.xml
  error: Requested operation is not valid: SCSI device 3:0:15:1074151456 is 
already in use by other domain(s) as 'non-shareable'

Seems that the virReportError issued for an internal error
overwrites one that might have already existed.  Rather than
remove them altogether (there may be error paths that don't
spit out a more helpful message), I thought maybe it'd be
best to check if one exists, and exit if one does.  If not,
the existing internal error messages are probably helpful.

Make this adjustment for both virtio-scsi and vhost-scsi
devices.

Signed-off-by: Eric Farman 
Reviewed-by: Bjoern Walk 
Reviewed-by: Boris Fiuczynski 
---
 src/qemu/qemu_hotplug.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 120bcdc..d421a77 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2470,6 +2470,10 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
 if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name,
   , 1)) {
 virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
+
+if (virGetLastError())
+return -1;
+
 if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
 virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = >u.iscsi;
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2595,9 +2599,12 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver,
 
 if (qemuHostdevPrepareSCSIVHostDevices(driver, vm->def->name, , 1) 
< 0) {
 virDomainHostdevSubsysSCSIVHostPtr hostsrc = 
>source.subsys.u.scsi_host;
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unable to prepare scsi_host hostdev: %s"),
-   hostsrc->wwpn);
+
+if (!virGetLastError()) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to prepare scsi_host hostdev: %s"),
+   hostsrc->wwpn);
+}
 return -1;
 }
 
-- 
1.9.1

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

[libvirt] [PATCH 11/14] secret: Alter FindByUUID to expect the formatted uuidstr

[John Ferlan]

Since we're storing a virUUIDFormat'd string in our Hash Table, let's
modify the Lookup API to receive a formatted string as well.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c| 18 +++---
 src/conf/virsecretobj.h|  2 +-
 src/secret/secret_driver.c | 10 +-
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 5acda4c..ae2b287 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -163,12 +163,8 @@ virSecretObjListDispose(void *obj)
 
 static virSecretObjPtr
 virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
- const unsigned char *uuid)
+ const char *uuidstr)
 {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-virUUIDFormat(uuid, uuidstr);
-
 return virObjectRef(virHashLookup(secrets->objs, uuidstr));
 }
 
@@ -176,7 +172,7 @@ virSecretObjListFindByUUIDLocked(virSecretObjListPtr 
secrets,
 /**
  * virSecretObjFindByUUID:
  * @secrets: list of secret objects
- * @uuid: secret uuid to find
+ * @uuidstr: secret uuid to find
  *
  * This function locks @secrets and finds the secret object which
  * corresponds to @uuid.
@@ -185,12 +181,12 @@ virSecretObjListFindByUUIDLocked(virSecretObjListPtr 
secrets,
  */
 virSecretObjPtr
 virSecretObjListFindByUUID(virSecretObjListPtr secrets,
-   const unsigned char *uuid)
+   const char *uuidstr)
 {
 virSecretObjPtr obj;
 
 virObjectLock(secrets);
-obj = virSecretObjListFindByUUIDLocked(secrets, uuid);
+obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr);
 virObjectUnlock(secrets);
 if (obj)
 virObjectLock(obj);
@@ -328,13 +324,14 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
 if (oldDef)
 *oldDef = NULL;
 
+virUUIDFormat(newdef->uuid, uuidstr);
+
 /* Is there a secret already matching this UUID */
-if ((obj = virSecretObjListFindByUUIDLocked(secrets, newdef->uuid))) {
+if ((obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr))) {
 virObjectLock(obj);
 def = obj->def;
 
 if (STRNEQ_NULLABLE(def->usage_id, newdef->usage_id)) {
-virUUIDFormat(def->uuid, uuidstr);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("a secret with UUID %s is already defined for "
  "use with %s"),
@@ -372,7 +369,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
 /* Generate the possible configFile and base64File strings
  * using the configDir, uuidstr, and appropriate suffix
  */
-virUUIDFormat(newdef->uuid, uuidstr);
 if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
 !(base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
 goto cleanup;
diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
index bd38f52..092f23c 100644
--- a/src/conf/virsecretobj.h
+++ b/src/conf/virsecretobj.h
@@ -40,7 +40,7 @@ virSecretObjListNew(void);
 
 virSecretObjPtr
 virSecretObjListFindByUUID(virSecretObjListPtr secrets,
-   const unsigned char *uuid);
+   const char *uuidstr);
 
 virSecretObjPtr
 virSecretObjListFindByUsage(virSecretObjListPtr secrets,
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index cc050ff..2d4091d 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -86,8 +86,8 @@ secretObjFromSecret(virSecretPtr secret)
 virSecretObjPtr obj;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-if (!(obj = virSecretObjListFindByUUID(driver->secrets, secret->uuid))) {
-virUUIDFormat(secret->uuid, uuidstr);
+virUUIDFormat(secret->uuid, uuidstr);
+if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuidstr))) {
 virReportError(VIR_ERR_NO_SECRET,
_("no secret with matching uuid '%s'"), uuidstr);
 return NULL;
@@ -159,10 +159,10 @@ secretLookupByUUID(virConnectPtr conn,
 virSecretPtr ret = NULL;
 virSecretObjPtr obj;
 virSecretDefPtr def;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuid))) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(uuid, uuidstr);
+virUUIDFormat(uuid, uuidstr);
+if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuidstr))) {
 virReportError(VIR_ERR_NO_SECRET,
_("no secret with matching uuid '%s'"), uuidstr);
 goto cleanup;
-- 
2.9.3

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

[libvirt] [PATCH 14/14] secret: Clean up virSecretObjListExport logic

[John Ferlan]

Shorten the time needed to keep the list lock and alter the cleanup
path to be more of an error path.

Utilize the the virObjectListFree function to handle the calls for
virObjectUnref on each list element and the VIR_FREE of the list
instead of open coding it.

Change the name of the virHashForEach callback to match the name
of the Export function with the Callback added onto it.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 7a9908d..caed156 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -550,9 +550,9 @@ struct virSecretObjListData {
 };
 
 static int
-virSecretObjListPopulate(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
+virSecretObjListExportCallback(void *payload,
+   const void *name ATTRIBUTE_UNUSED,
+   void *opaque)
 {
 struct virSecretObjListData *data = opaque;
 virSecretObjPtr obj = payload;
@@ -598,7 +598,6 @@ virSecretObjListExport(virConnectPtr conn,
virSecretObjListACLFilter aclfilter,
unsigned int flags)
 {
-int ret = -1;
 struct virSecretObjListData data = {
 .conn = conn, .secrets = NULL,
 .aclfilter = aclfilter, .flags = flags,
@@ -606,30 +605,28 @@ virSecretObjListExport(virConnectPtr conn,
 
 virObjectLock(secretobjs);
 if (secrets &&
-VIR_ALLOC_N(data.secrets, virHashSize(secretobjs->objs) + 1) < 0)
-goto cleanup;
+VIR_ALLOC_N(data.secrets, virHashSize(secretobjs->objs) + 1) < 0) {
+virObjectUnlock(secretobjs);
+return -1;
+}
 
-virHashForEach(secretobjs->objs, virSecretObjListPopulate, );
+virHashForEach(secretobjs->objs, virSecretObjListExportCallback, );
+virObjectUnlock(secretobjs);
 
 if (data.error)
-goto cleanup;
+goto error;
 
 if (data.secrets) {
 /* trim the array to the final size */
 ignore_value(VIR_REALLOC_N(data.secrets, data.nsecrets + 1));
 *secrets = data.secrets;
-data.secrets = NULL;
 }
 
-ret = data.nsecrets;
+return data.nsecrets;
 
- cleanup:
-virObjectUnlock(secretobjs);
-while (data.secrets && data.nsecrets)
-virObjectUnref(data.secrets[--data.nsecrets]);
-
-VIR_FREE(data.secrets);
-return ret;
+ error:
+virObjectListFree(data.secrets);
+return -1;
 }
 
 
-- 
2.9.3

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

[libvirt] [PATCH 13/14] secret: Alter configFile/base64File mgmt

[John Ferlan]

Rather than being generated during virSecretObjListAdd, generate the file
paths in each of the callers and then copy those paths into the object
rather than stealing their pointers.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c| 53 --
 src/conf/virsecretobj.h|  3 ++-
 src/secret/secret_driver.c | 14 ++--
 3 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index ae2b287..7a9908d 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -300,7 +300,8 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
  * virSecretObjListAdd:
  * @secrets: list of secret objects
  * @newdef: new secret definition
- * @configDir: directory to place secret config files
+ * @configFile: secret config file
+ * @base64File: secret data file
  * @oldDef: Former secret def (e.g. a reload path perhaps)
  *
  * Add the new @newdef to the secret obj table hash
@@ -310,14 +311,14 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
 virSecretObjPtr
 virSecretObjListAdd(virSecretObjListPtr secrets,
 virSecretDefPtr newdef,
-const char *configDir,
+const char *configFile,
+const char *base64File,
 virSecretDefPtr *oldDef)
 {
 virSecretObjPtr obj;
 virSecretDefPtr def;
 virSecretObjPtr ret = NULL;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
-char *configFile = NULL, *base64File = NULL;
 
 virObjectLock(secrets);
 
@@ -366,13 +367,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
 goto cleanup;
 }
 
-/* Generate the possible configFile and base64File strings
- * using the configDir, uuidstr, and appropriate suffix
- */
-if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
-!(base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
-goto cleanup;
-
 if (!(obj = virSecretObjNew()))
 goto cleanup;
 
@@ -380,8 +374,10 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
 goto cleanup;
 
 obj->def = newdef;
-VIR_STEAL_PTR(obj->configFile, configFile);
-VIR_STEAL_PTR(obj->base64File, base64File);
+if ((VIR_STRDUP(obj->configFile, configFile) < 0) ||
+(VIR_STRDUP(obj->base64File, base64File) < 0))
+goto cleanup;
+
 virObjectRef(obj);
 }
 
@@ -390,8 +386,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
 
  cleanup:
 virSecretObjEndAPI();
-VIR_FREE(configFile);
-VIR_FREE(base64File);
 virObjectUnlock(secrets);
 return ret;
 }
@@ -899,21 +893,22 @@ virSecretLoadValue(virSecretObjPtr obj)
 
 static virSecretObjPtr
 virSecretLoad(virSecretObjListPtr secrets,
-  const char *file,
-  const char *path,
-  const char *configDir)
+  const char *fname,
+  const char *configFile,
+  const char *base64File)
 {
 virSecretDefPtr def = NULL;
 virSecretObjPtr obj = NULL;
 virSecretObjPtr ret = NULL;
 
-if (!(def = virSecretDefParseFile(path)))
+if (!(def = virSecretDefParseFile(configFile)))
 goto cleanup;
 
-if (virSecretLoadValidateUUID(def, file) < 0)
+if (virSecretLoadValidateUUID(def, fname) < 0)
 goto cleanup;
 
-if (!(obj = virSecretObjListAdd(secrets, def, configDir, NULL)))
+if (!(obj = virSecretObjListAdd(secrets, def, configFile, base64File,
+NULL)))
 goto cleanup;
 def = NULL;
 
@@ -936,6 +931,8 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
 {
 DIR *dir = NULL;
 struct dirent *de;
+char *configFile = NULL;
+char *base64File = NULL;
 int rc;
 
 if ((rc = virDirOpenIfExists(, configDir)) <= 0)
@@ -944,26 +941,32 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
 /* Ignore errors reported by readdir or other calls within the
  * loop (if any).  It's better to keep the secrets we managed to find. */
 while (virDirRead(dir, , NULL) > 0) {
-char *path;
 virSecretObjPtr obj;
 
+VIR_FREE(configFile);
+VIR_FREE(base64File);
+
 if (!virFileHasSuffix(de->d_name, ".xml"))
 continue;
 
-if (!(path = virFileBuildPath(configDir, de->d_name, NULL)))
+if (!(configFile = virFileBuildPath(configDir, de->d_name, ".xml")))
+continue;
+
+if (!(base64File = virFileBuildPath(configDir, de->d_name, "base64")))
 continue;
 
-if (!(obj = virSecretLoad(secrets, de->d_name, path, configDir))) {
+if (!(obj = virSecretLoad(secrets, de->d_name, configFile,
+  base64File))) {
 VIR_ERROR(_("Error reading secret: %s"),
   virGetLastErrorMessage());
-VIR_FREE(path);
 

[libvirt] [PATCH 09/14] secret: Split apart NumOfSecrets and GetUUIDs callback function

[John Ferlan]

Rather than overloading one function - split apart the logic to have
separate interfaces and local/private structures to manage the data
for which the helper is collecting.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 98 +++--
 src/conf/virsecretobj.h |  6 +--
 2 files changed, 65 insertions(+), 39 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index c410a6b..3717552 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -415,9 +415,54 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
 }
 
 
-struct virSecretObjListGetHelperData {
+struct secretCountData {
 virConnectPtr conn;
-virSecretObjListACLFilter filter;
+virSecretObjListACLFilter aclfilter;
+int count;
+};
+
+static int
+virSecretObjListNumOfSecretsCallback(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+struct secretCountData *data = opaque;
+virSecretObjPtr obj = payload;
+virSecretDefPtr def;
+
+virObjectLock(obj);
+def = obj->def;
+
+if (data->aclfilter && !data->aclfilter(data->conn, def))
+goto cleanup;
+
+data->count++;
+
+ cleanup:
+virObjectUnlock(obj);
+return 0;
+}
+
+
+int
+virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
+ virSecretObjListACLFilter aclfilter,
+ virConnectPtr conn)
+{
+struct secretCountData data = {
+.conn = conn, .aclfilter = aclfilter, .count = 0 };
+
+virObjectLock(secrets);
+virHashForEach(secrets->objs, virSecretObjListNumOfSecretsCallback, );
+virObjectUnlock(secrets);
+
+return data.count;
+}
+
+
+struct secretListData {
+virConnectPtr conn;
+virSecretObjListACLFilter aclfilter;
 int nuuids;
 char **uuids;
 int maxuuids;
@@ -426,11 +471,11 @@ struct virSecretObjListGetHelperData {
 
 
 static int
-virSecretObjListGetHelper(void *payload,
-  const void *name ATTRIBUTE_UNUSED,
-  void *opaque)
+virSecretObjListGetUUIDsCallback(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
 {
-struct virSecretObjListGetHelperData *data = opaque;
+struct secretListData *data = opaque;
 virSecretObjPtr obj = payload;
 virSecretDefPtr def;
 
@@ -443,7 +488,7 @@ virSecretObjListGetHelper(void *payload,
 virObjectLock(obj);
 def = obj->def;
 
-if (data->filter && !data->filter(data->conn, def))
+if (data->aclfilter && !data->aclfilter(data->conn, def))
 goto cleanup;
 
 if (data->uuids) {
@@ -455,11 +500,9 @@ virSecretObjListGetHelper(void *payload,
 }
 
 virUUIDFormat(def->uuid, uuidstr);
-data->uuids[data->nuuids] = uuidstr;
+data->uuids[data->nuuids++] = uuidstr;
 }
 
-data->nuuids++;
-
  cleanup:
 virObjectUnlock(obj);
 return 0;
@@ -467,35 +510,18 @@ virSecretObjListGetHelper(void *payload,
 
 
 int
-virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
- virSecretObjListACLFilter filter,
- virConnectPtr conn)
-{
-struct virSecretObjListGetHelperData data = {
-.conn = conn, .filter = filter, .nuuids = 0,
-.uuids = NULL, .maxuuids = -1, .error = false };
-
-virObjectLock(secrets);
-virHashForEach(secrets->objs, virSecretObjListGetHelper, );
-virObjectUnlock(secrets);
-
-return data.nuuids;
-}
-
-
-int
 virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
  char **uuids,
  int maxuuids,
- virSecretObjListACLFilter filter,
+ virSecretObjListACLFilter aclfilter,
  virConnectPtr conn)
 {
-struct virSecretObjListGetHelperData data = {
-.conn = conn, .filter = filter, .nuuids = 0,
-.uuids = uuids, .maxuuids = maxuuids, .error = false };
+struct secretListData data = {
+.conn = conn, .aclfilter = aclfilter, .uuids = uuids, .nuuids = 0,
+.maxuuids = maxuuids, .error = false };
 
 virObjectLock(secrets);
-virHashForEach(secrets->objs, virSecretObjListGetHelper, );
+virHashForEach(secrets->objs, virSecretObjListGetUUIDsCallback, );
 virObjectUnlock(secrets);
 
 if (data.error)
@@ -541,7 +567,7 @@ virSecretObjMatchFlags(virSecretObjPtr obj,
 struct virSecretObjListData {
 virConnectPtr conn;
 virSecretPtr *secrets;
-virSecretObjListACLFilter filter;
+virSecretObjListACLFilter aclfilter;
 unsigned int flags;
 int nsecrets;
 bool error;
@@ -563,7 +589,7 @@ virSecretObjListPopulate(void *payload,
 virObjectLock(obj);
 def = obj->def;
 
-if (data->filter && !data->filter(data->conn, def))
+if (data->aclfilter && 

[libvirt] [PATCH 08/14] secret: Change variable names for list traversals

[John Ferlan]

Rather than 'nuuids' it should be 'maxuuids' and rather than 'got'
it should be 'nuuids'.  Alter the logic of the list traversal to
utilize those names.

Also alter the cleanup in virSecretObjListGetUUIDs to return on success
and handle failure with a -1 return instead of the if (ret < 0) logic.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 38 +-
 src/conf/virsecretobj.h |  2 +-
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 998f815..c410a6b 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -418,9 +418,9 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
 struct virSecretObjListGetHelperData {
 virConnectPtr conn;
 virSecretObjListACLFilter filter;
-int got;
-char **uuids;
 int nuuids;
+char **uuids;
+int maxuuids;
 bool error;
 };
 
@@ -437,7 +437,7 @@ virSecretObjListGetHelper(void *payload,
 if (data->error)
 return 0;
 
-if (data->nuuids >= 0 && data->got == data->nuuids)
+if (data->maxuuids >= 0 && data->nuuids == data->maxuuids)
 return 0;
 
 virObjectLock(obj);
@@ -455,10 +455,10 @@ virSecretObjListGetHelper(void *payload,
 }
 
 virUUIDFormat(def->uuid, uuidstr);
-data->uuids[data->got] = uuidstr;
+data->uuids[data->nuuids] = uuidstr;
 }
 
-data->got++;
+data->nuuids++;
 
  cleanup:
 virObjectUnlock(obj);
@@ -472,45 +472,41 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
  virConnectPtr conn)
 {
 struct virSecretObjListGetHelperData data = {
-.conn = conn, .filter = filter, .got = 0,
-.uuids = NULL, .nuuids = -1, .error = false };
+.conn = conn, .filter = filter, .nuuids = 0,
+.uuids = NULL, .maxuuids = -1, .error = false };
 
 virObjectLock(secrets);
 virHashForEach(secrets->objs, virSecretObjListGetHelper, );
 virObjectUnlock(secrets);
 
-return data.got;
+return data.nuuids;
 }
 
 
 int
 virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
  char **uuids,
- int nuuids,
+ int maxuuids,
  virSecretObjListACLFilter filter,
  virConnectPtr conn)
 {
-int ret = -1;
-
 struct virSecretObjListGetHelperData data = {
-.conn = conn, .filter = filter, .got = 0,
-.uuids = uuids, .nuuids = nuuids, .error = false };
+.conn = conn, .filter = filter, .nuuids = 0,
+.uuids = uuids, .maxuuids = maxuuids, .error = false };
 
 virObjectLock(secrets);
 virHashForEach(secrets->objs, virSecretObjListGetHelper, );
 virObjectUnlock(secrets);
 
 if (data.error)
-goto cleanup;
+goto error;
 
-ret = data.got;
+return data.nuuids;
 
- cleanup:
-if (ret < 0) {
-while (data.got)
-VIR_FREE(data.uuids[--data.got]);
-}
-return ret;
+ error:
+while (--data.nuuids)
+VIR_FREE(data.uuids[data.nuuids]);
+return -1;
 }
 
 
diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
index 82915d0..7582913 100644
--- a/src/conf/virsecretobj.h
+++ b/src/conf/virsecretobj.h
@@ -69,7 +69,7 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
 int
 virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
  char **uuids,
- int nuuids,
+ int maxuuids,
  virSecretObjListACLFilter filter,
  virConnectPtr conn);
 
-- 
2.9.3

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

[libvirt] [PATCH 12/14] secret: Generate configDir during driver initialization

[John Ferlan]

Rather than waiting for the first save to fail, let's generate the
directory with the correct privs during initialization

Signed-off-by: John Ferlan 
---
 src/secret/secret_driver.c | 23 ++-
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 2d4091d..8ddae57 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -96,17 +96,6 @@ secretObjFromSecret(virSecretPtr secret)
 }
 
 
-static int
-secretEnsureDirectory(void)
-{
-if (mkdir(driver->configDir, S_IRWXU) < 0 && errno != EEXIST) {
-virReportSystemError(errno, _("cannot create '%s'"),
- driver->configDir);
-return -1;
-}
-return 0;
-}
-
 /* Driver functions */
 
 static int
@@ -238,9 +227,6 @@ secretDefineXML(virConnectPtr conn,
 goto cleanup;
 
 if (!def->isephemeral) {
-if (secretEnsureDirectory() < 0)
-goto cleanup;
-
 if (backup && backup->isephemeral) {
 if (virSecretObjSaveData(obj) < 0)
 goto restore_backup;
@@ -341,9 +327,6 @@ secretSetValue(virSecretPtr secret,
 if (virSecretSetValueEnsureACL(secret->conn, def) < 0)
 goto cleanup;
 
-if (secretEnsureDirectory() < 0)
-goto cleanup;
-
 if (virSecretObjSetValue(obj, value, value_size) < 0)
 goto cleanup;
 
@@ -488,6 +471,12 @@ secretStateInitialize(bool privileged,
 goto error;
 VIR_FREE(base);
 
+if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) {
+virReportSystemError(errno, _("cannot create config directory '%s'"),
+ driver->configDir);
+goto error;
+}
+
 if (!(driver->secrets = virSecretObjListNew()))
 goto error;
 
-- 
2.9.3

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

[libvirt] [PATCH 06/14] secret: Use virSecretDefPtr rather than deref from virSecretObjPtr

[John Ferlan]

Rather than dereferencing obj->def->X, create a local 'def' variable
variable that will dereference the def and use directly.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 69 +++--
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 42f36c8..413955d 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -139,8 +139,9 @@ static void
 virSecretObjDispose(void *opaque)
 {
 virSecretObjPtr obj = opaque;
+virSecretDefPtr def = obj->def;
 
-virSecretDefFree(obj->def);
+virSecretDefFree(def);
 if (obj->value) {
 /* Wipe before free to ensure we don't leave a secret on the heap */
 memset(obj->value, 0, obj->value_size);
@@ -203,16 +204,18 @@ virSecretObjSearchName(const void *payload,
const void *opaque)
 {
 virSecretObjPtr obj = (virSecretObjPtr) payload;
+virSecretDefPtr def;
 struct virSecretSearchData *data = (struct virSecretSearchData *) opaque;
 int found = 0;
 
 virObjectLock(obj);
+def = obj->def;
 
-if (obj->def->usage_type != data->usageType)
+if (def->usage_type != data->usageType)
 goto cleanup;
 
 if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE &&
-STREQ(obj->def->usage_id, data->usageID))
+STREQ(def->usage_id, data->usageID))
 found = 1;
 
  cleanup:
@@ -278,11 +281,13 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
virSecretObjPtr obj)
 {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
+virSecretDefPtr def;
 
 if (!obj)
 return;
+def = obj->def;
 
-virUUIDFormat(obj->def->uuid, uuidstr);
+virUUIDFormat(def->uuid, uuidstr);
 virObjectRef(obj);
 virObjectUnlock(obj);
 
@@ -315,6 +320,7 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
   virSecretDefPtr *oldDef)
 {
 virSecretObjPtr obj;
+virSecretDefPtr def;
 virSecretObjPtr ret = NULL;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 char *configFile = NULL, *base64File = NULL;
@@ -325,26 +331,27 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
 /* Is there a secret already matching this UUID */
 if ((obj = virSecretObjListFindByUUIDLocked(secrets, newdef->uuid))) {
 virObjectLock(obj);
+def = obj->def;
 
-if (STRNEQ_NULLABLE(obj->def->usage_id, newdef->usage_id)) {
-virUUIDFormat(obj->def->uuid, uuidstr);
+if (STRNEQ_NULLABLE(def->usage_id, newdef->usage_id)) {
+virUUIDFormat(def->uuid, uuidstr);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("a secret with UUID %s is already defined for "
  "use with %s"),
-   uuidstr, obj->def->usage_id);
+   uuidstr, def->usage_id);
 goto cleanup;
 }
 
-if (obj->def->isprivate && !newdef->isprivate) {
+if (def->isprivate && !newdef->isprivate) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cannot change private flag on existing secret"));
 goto cleanup;
 }
 
 if (oldDef)
-*oldDef = obj->def;
+*oldDef = def;
 else
-virSecretDefFree(obj->def);
+virSecretDefFree(def);
 obj->def = newdef;
 } else {
 /* No existing secret with same UUID,
@@ -353,7 +360,8 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
  newdef->usage_type,
  newdef->usage_id))) {
 virObjectLock(obj);
-virUUIDFormat(obj->def->uuid, uuidstr);
+def = obj->def;
+virUUIDFormat(def->uuid, uuidstr);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("a secret with UUID %s already defined for "
  "use with %s"),
@@ -424,6 +432,7 @@ virSecretObjListGetHelper(void *payload,
 {
 struct virSecretObjListGetHelperData *data = opaque;
 virSecretObjPtr obj = payload;
+virSecretDefPtr def;
 
 if (data->error)
 return 0;
@@ -432,8 +441,9 @@ virSecretObjListGetHelper(void *payload,
 return 0;
 
 virObjectLock(obj);
+def = obj->def;
 
-if (data->filter && !data->filter(data->conn, obj->def))
+if (data->filter && !data->filter(data->conn, def))
 goto cleanup;
 
 if (data->uuids) {
@@ -444,7 +454,7 @@ virSecretObjListGetHelper(void *payload,
 goto cleanup;
 }
 
-virUUIDFormat(obj->def->uuid, uuidstr);
+virUUIDFormat(def->uuid, uuidstr);
 data->uuids[data->got] = uuidstr;
 }
 
@@ -478,20 +488,22 @@ static bool
 virSecretObjMatchFlags(virSecretObjPtr obj,
unsigned int flags)
 

[libvirt] [PATCH 10/14] secret: Combine virSecretObjListAdd with Locked function

[John Ferlan]

There's no need to separate, so just have one.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 34 ++
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 3717552..5acda4c 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -301,7 +301,7 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
 
 
 /*
- * virSecretObjListAddLocked:
+ * virSecretObjListAdd:
  * @secrets: list of secret objects
  * @newdef: new secret definition
  * @configDir: directory to place secret config files
@@ -309,15 +309,13 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
  *
  * Add the new @newdef to the secret obj table hash
  *
- * This functions requires @secrets to be locked already!
- *
- * Returns: locked secret or NULL if failure to add
+ * Returns: locked and ref'd secret or NULL if failure to add
  */
-static virSecretObjPtr
-virSecretObjListAddLocked(virSecretObjListPtr secrets,
-  virSecretDefPtr newdef,
-  const char *configDir,
-  virSecretDefPtr *oldDef)
+virSecretObjPtr
+virSecretObjListAdd(virSecretObjListPtr secrets,
+virSecretDefPtr newdef,
+const char *configDir,
+virSecretDefPtr *oldDef)
 {
 virSecretObjPtr obj;
 virSecretDefPtr def;
@@ -325,6 +323,8 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 char *configFile = NULL, *base64File = NULL;
 
+virObjectLock(secrets);
+
 if (oldDef)
 *oldDef = NULL;
 
@@ -396,22 +396,8 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
 virSecretObjEndAPI();
 VIR_FREE(configFile);
 VIR_FREE(base64File);
-return ret;
-}
-
-
-virSecretObjPtr
-virSecretObjListAdd(virSecretObjListPtr secrets,
-virSecretDefPtr newdef,
-const char *configDir,
-virSecretDefPtr *oldDef)
-{
-virSecretObjPtr obj;
-
-virObjectLock(secrets);
-obj = virSecretObjListAddLocked(secrets, newdef, configDir, oldDef);
 virObjectUnlock(secrets);
-return obj;
+return ret;
 }
 
 
-- 
2.9.3

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

[libvirt] [PATCH 07/14] secret: Move virSecretObjListGetUUIDs

[John Ferlan]

Move code closer to usage.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 62 -
 src/conf/virsecretobj.h | 14 +--
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 413955d..998f815 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -483,6 +483,37 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
 }
 
 
+int
+virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
+ char **uuids,
+ int nuuids,
+ virSecretObjListACLFilter filter,
+ virConnectPtr conn)
+{
+int ret = -1;
+
+struct virSecretObjListGetHelperData data = {
+.conn = conn, .filter = filter, .got = 0,
+.uuids = uuids, .nuuids = nuuids, .error = false };
+
+virObjectLock(secrets);
+virHashForEach(secrets->objs, virSecretObjListGetHelper, );
+virObjectUnlock(secrets);
+
+if (data.error)
+goto cleanup;
+
+ret = data.got;
+
+ cleanup:
+if (ret < 0) {
+while (data.got)
+VIR_FREE(data.uuids[--data.got]);
+}
+return ret;
+}
+
+
 #define MATCH(FLAG) (flags & (FLAG))
 static bool
 virSecretObjMatchFlags(virSecretObjPtr obj,
@@ -605,37 +636,6 @@ virSecretObjListExport(virConnectPtr conn,
 
 
 int
-virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
- char **uuids,
- int nuuids,
- virSecretObjListACLFilter filter,
- virConnectPtr conn)
-{
-int ret = -1;
-
-struct virSecretObjListGetHelperData data = {
-.conn = conn, .filter = filter, .got = 0,
-.uuids = uuids, .nuuids = nuuids, .error = false };
-
-virObjectLock(secrets);
-virHashForEach(secrets->objs, virSecretObjListGetHelper, );
-virObjectUnlock(secrets);
-
-if (data.error)
-goto cleanup;
-
-ret = data.got;
-
- cleanup:
-if (ret < 0) {
-while (data.got)
-VIR_FREE(data.uuids[--data.got]);
-}
-return ret;
-}
-
-
-int
 virSecretObjDeleteConfig(virSecretObjPtr obj)
 {
 virSecretDefPtr def = obj->def;
diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
index 8038faa..82915d0 100644
--- a/src/conf/virsecretobj.h
+++ b/src/conf/virsecretobj.h
@@ -67,13 +67,6 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
  virConnectPtr conn);
 
 int
-virSecretObjListExport(virConnectPtr conn,
-   virSecretObjListPtr secretobjs,
-   virSecretPtr **secrets,
-   virSecretObjListACLFilter filter,
-   unsigned int flags);
-
-int
 virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
  char **uuids,
  int nuuids,
@@ -81,6 +74,13 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
  virConnectPtr conn);
 
 int
+virSecretObjListExport(virConnectPtr conn,
+   virSecretObjListPtr secretobjs,
+   virSecretPtr **secrets,
+   virSecretObjListACLFilter filter,
+   unsigned int flags);
+
+int
 virSecretObjDeleteConfig(virSecretObjPtr obj);
 
 void
-- 
2.9.3

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

[libvirt] [PATCH 02/14] secret: Convert virsecretobjs.h to use "newer" formatting style

[John Ferlan]

Alter the prototypes to use the newer formatting style

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.h | 147 +---
 1 file changed, 88 insertions(+), 59 deletions(-)

diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
index fa45b42..b26061a 100644
--- a/src/conf/virsecretobj.h
+++ b/src/conf/virsecretobj.h
@@ -29,82 +29,111 @@
 typedef struct _virSecretObj virSecretObj;
 typedef virSecretObj *virSecretObjPtr;
 
-virSecretObjPtr virSecretObjNew(void);
+virSecretObjPtr
+virSecretObjNew(void);
 
-void virSecretObjEndAPI(virSecretObjPtr *secret);
+void
+virSecretObjEndAPI(virSecretObjPtr *secret);
 
 typedef struct _virSecretObjList virSecretObjList;
 typedef virSecretObjList *virSecretObjListPtr;
 
-virSecretObjListPtr virSecretObjListNew(void);
-
-virSecretObjPtr virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
- const unsigned char *uuid);
-
-virSecretObjPtr virSecretObjListFindByUUID(virSecretObjListPtr secrets,
-   const unsigned char *uuid);
-
-virSecretObjPtr virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
-  int usageType,
-  const char *usageID);
-
-virSecretObjPtr virSecretObjListFindByUsage(virSecretObjListPtr secrets,
-int usageType,
-const char *usageID);
-
-void virSecretObjListRemove(virSecretObjListPtr secrets,
-virSecretObjPtr secret);
-
-virSecretObjPtr virSecretObjListAddLocked(virSecretObjListPtr secrets,
-  virSecretDefPtr def,
-  const char *configDir,
-  virSecretDefPtr *oldDef);
-
-virSecretObjPtr virSecretObjListAdd(virSecretObjListPtr secrets,
-virSecretDefPtr def,
-const char *configDir,
-virSecretDefPtr *oldDef);
-
-typedef bool (*virSecretObjListACLFilter)(virConnectPtr conn,
-  virSecretDefPtr def);
+virSecretObjListPtr
+virSecretObjListNew(void);
+
+virSecretObjPtr
+virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
+ const unsigned char *uuid);
+
+virSecretObjPtr
+virSecretObjListFindByUUID(virSecretObjListPtr secrets,
+   const unsigned char *uuid);
+
+virSecretObjPtr
+virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
+  int usageType,
+  const char *usageID);
+
+virSecretObjPtr
+virSecretObjListFindByUsage(virSecretObjListPtr secrets,
+int usageType,
+const char *usageID);
+
+void
+virSecretObjListRemove(virSecretObjListPtr secrets,
+   virSecretObjPtr secret);
+
+virSecretObjPtr
+virSecretObjListAddLocked(virSecretObjListPtr secrets,
+  virSecretDefPtr def,
+  const char *configDir,
+  virSecretDefPtr *oldDef);
+
+virSecretObjPtr
+virSecretObjListAdd(virSecretObjListPtr secrets,
+virSecretDefPtr def,
+const char *configDir,
+virSecretDefPtr *oldDef);
+
+typedef bool
+(*virSecretObjListACLFilter)(virConnectPtr conn,
+ virSecretDefPtr def);
+
+int
+virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
+ virSecretObjListACLFilter filter,
+ virConnectPtr conn);
 
-int virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
- virSecretObjListACLFilter filter,
- virConnectPtr conn);
+int
+virSecretObjListExport(virConnectPtr conn,
+   virSecretObjListPtr secretobjs,
+   virSecretPtr **secrets,
+   virSecretObjListACLFilter filter,
+   unsigned int flags);
 
-int virSecretObjListExport(virConnectPtr conn,
-   virSecretObjListPtr secretobjs,
-   virSecretPtr **secrets,
-   virSecretObjListACLFilter filter,
-   unsigned int flags);
+int
+virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
+ char **uuids,
+ int nuuids,
+ virSecretObjListACLFilter filter,
+ virConnectPtr conn);
 
-int virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
- char **uuids,
- int nuuids,
- 

[libvirt] [PATCH 04/14] secret: Have virSecretObjNew return locked object

[John Ferlan]

Rather than have caller need to do it, have the object returned locked.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 064e66c..348feb3 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -97,6 +97,8 @@ virSecretObjNew(void)
 if (!(secret = virObjectLockableNew(virSecretObjClass)))
 return NULL;
 
+virObjectLock(secret);
+
 return secret;
 }
 
@@ -367,8 +369,6 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
 if (!(secret = virSecretObjNew()))
 goto cleanup;
 
-virObjectLock(secret);
-
 if (virHashAddEntry(secrets->objs, uuidstr, secret) < 0)
 goto cleanup;
 
-- 
2.9.3

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

[libvirt] [PATCH 03/14] secret: Make some virSecretObj* functions static

[John Ferlan]

Make various virSecretObjList*Locked functions static and make
virSecretObjNew static since they're only called within virtsecretobj.c

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 33 +++--
 src/conf/virsecretobj.h | 18 --
 2 files changed, 7 insertions(+), 44 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index cc18459..064e66c 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -86,7 +86,7 @@ virSecretObjOnceInit(void)
 
 VIR_ONCE_GLOBAL_INIT(virSecretObj)
 
-virSecretObjPtr
+static virSecretObjPtr
 virSecretObjNew(void)
 {
 virSecretObjPtr secret;
@@ -158,16 +158,7 @@ virSecretObjListDispose(void *obj)
 }
 
 
-/**
- * virSecretObjFindByUUIDLocked:
- * @secrets: list of secret objects
- * @uuid: secret uuid to find
- *
- * This functions requires @secrets to be locked already!
- *
- * Returns: not locked, but ref'd secret object.
- */
-virSecretObjPtr
+static virSecretObjPtr
 virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
  const unsigned char *uuid)
 {
@@ -187,7 +178,7 @@ virSecretObjListFindByUUIDLocked(virSecretObjListPtr 
secrets,
  * This function locks @secrets and finds the secret object which
  * corresponds to @uuid.
  *
- * Returns: locked and ref'd secret object.
+ * Returns: locked and ref'd secret object on success, NULL on failure.
  */
 virSecretObjPtr
 virSecretObjListFindByUUID(virSecretObjListPtr secrets,
@@ -228,17 +219,7 @@ virSecretObjSearchName(const void *payload,
 }
 
 
-/**
- * virSecretObjFindByUsageLocked:
- * @secrets: list of secret objects
- * @usageType: secret usageType to find
- * @usageID: secret usage string
- *
- * This functions requires @secrets to be locked already!
- *
- * Returns: not locked, but ref'd secret object.
- */
-virSecretObjPtr
+static virSecretObjPtr
 virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
   int usageType,
   const char *usageID)
@@ -263,7 +244,7 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr 
secrets,
  * This function locks @secrets and finds the secret object which
  * corresponds to @usageID of @usageType.
  *
- * Returns: locked and ref'd secret object.
+ * Returns: locked and ref'd secret object on success, NULL on failure.
  */
 virSecretObjPtr
 virSecretObjListFindByUsage(virSecretObjListPtr secrets,
@@ -320,9 +301,9 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
  *
  * This functions requires @secrets to be locked already!
  *
- * Returns pointer to secret or NULL if failure to add
+ * Returns: locked secret or NULL if failure to add
  */
-virSecretObjPtr
+static virSecretObjPtr
 virSecretObjListAddLocked(virSecretObjListPtr secrets,
   virSecretDefPtr def,
   const char *configDir,
diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
index b26061a..9638b69 100644
--- a/src/conf/virsecretobj.h
+++ b/src/conf/virsecretobj.h
@@ -29,9 +29,6 @@
 typedef struct _virSecretObj virSecretObj;
 typedef virSecretObj *virSecretObjPtr;
 
-virSecretObjPtr
-virSecretObjNew(void);
-
 void
 virSecretObjEndAPI(virSecretObjPtr *secret);
 
@@ -42,19 +39,10 @@ virSecretObjListPtr
 virSecretObjListNew(void);
 
 virSecretObjPtr
-virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
- const unsigned char *uuid);
-
-virSecretObjPtr
 virSecretObjListFindByUUID(virSecretObjListPtr secrets,
const unsigned char *uuid);
 
 virSecretObjPtr
-virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
-  int usageType,
-  const char *usageID);
-
-virSecretObjPtr
 virSecretObjListFindByUsage(virSecretObjListPtr secrets,
 int usageType,
 const char *usageID);
@@ -64,12 +52,6 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
virSecretObjPtr secret);
 
 virSecretObjPtr
-virSecretObjListAddLocked(virSecretObjListPtr secrets,
-  virSecretDefPtr def,
-  const char *configDir,
-  virSecretDefPtr *oldDef);
-
-virSecretObjPtr
 virSecretObjListAdd(virSecretObjListPtr secrets,
 virSecretDefPtr def,
 const char *configDir,
-- 
2.9.3

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

[libvirt] [PATCH 05/14] secret: Use consistent naming for variables

[John Ferlan]

When processing a virSecretPtr use 'secret' as a variable name.

When processing a virSecretObjPtr use 'obj' as a variable name.

When processing a virSecretDefPtr use 'def' as a variable name,
unless a distinction needs to be made with a 'newdef' such as
virSecretObjListAddLocked (which also used the VIR_STEAL_PTR macro
for the configFile and base64File).

NB: Also made a slight algorithm adjustment for virSecretObjListRemove
to check if the passed obj was NULL rather than having the caller
virSecretLoad need to check prior to calling.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c| 275 +++--
 src/conf/virsecretobj.h|  26 ++---
 src/secret/secret_driver.c | 139 ---
 3 files changed, 226 insertions(+), 214 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 348feb3..42f36c8 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -89,29 +89,29 @@ VIR_ONCE_GLOBAL_INIT(virSecretObj)
 static virSecretObjPtr
 virSecretObjNew(void)
 {
-virSecretObjPtr secret;
+virSecretObjPtr obj;
 
 if (virSecretObjInitialize() < 0)
 return NULL;
 
-if (!(secret = virObjectLockableNew(virSecretObjClass)))
+if (!(obj = virObjectLockableNew(virSecretObjClass)))
 return NULL;
 
-virObjectLock(secret);
+virObjectLock(obj);
 
-return secret;
+return obj;
 }
 
 
 void
-virSecretObjEndAPI(virSecretObjPtr *secret)
+virSecretObjEndAPI(virSecretObjPtr *obj)
 {
-if (!*secret)
+if (!*obj)
 return;
 
-virObjectUnlock(*secret);
-virObjectUnref(*secret);
-*secret = NULL;
+virObjectUnlock(*obj);
+virObjectUnref(*obj);
+*obj = NULL;
 }
 
 
@@ -136,18 +136,18 @@ virSecretObjListNew(void)
 
 
 static void
-virSecretObjDispose(void *obj)
+virSecretObjDispose(void *opaque)
 {
-virSecretObjPtr secret = obj;
+virSecretObjPtr obj = opaque;
 
-virSecretDefFree(secret->def);
-if (secret->value) {
+virSecretDefFree(obj->def);
+if (obj->value) {
 /* Wipe before free to ensure we don't leave a secret on the heap */
-memset(secret->value, 0, secret->value_size);
-VIR_FREE(secret->value);
+memset(obj->value, 0, obj->value_size);
+VIR_FREE(obj->value);
 }
-VIR_FREE(secret->configFile);
-VIR_FREE(secret->base64File);
+VIR_FREE(obj->configFile);
+VIR_FREE(obj->base64File);
 }
 
 
@@ -186,14 +186,14 @@ virSecretObjPtr
 virSecretObjListFindByUUID(virSecretObjListPtr secrets,
const unsigned char *uuid)
 {
-virSecretObjPtr ret;
+virSecretObjPtr obj;
 
 virObjectLock(secrets);
-ret = virSecretObjListFindByUUIDLocked(secrets, uuid);
+obj = virSecretObjListFindByUUIDLocked(secrets, uuid);
 virObjectUnlock(secrets);
-if (ret)
-virObjectLock(ret);
-return ret;
+if (obj)
+virObjectLock(obj);
+return obj;
 }
 
 
@@ -202,21 +202,21 @@ virSecretObjSearchName(const void *payload,
const void *name ATTRIBUTE_UNUSED,
const void *opaque)
 {
-virSecretObjPtr secret = (virSecretObjPtr) payload;
+virSecretObjPtr obj = (virSecretObjPtr) payload;
 struct virSecretSearchData *data = (struct virSecretSearchData *) opaque;
 int found = 0;
 
-virObjectLock(secret);
+virObjectLock(obj);
 
-if (secret->def->usage_type != data->usageType)
+if (obj->def->usage_type != data->usageType)
 goto cleanup;
 
 if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE &&
-STREQ(secret->def->usage_id, data->usageID))
+STREQ(obj->def->usage_id, data->usageID))
 found = 1;
 
  cleanup:
-virObjectUnlock(secret);
+virObjectUnlock(obj);
 return found;
 }
 
@@ -226,14 +226,14 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr 
secrets,
   int usageType,
   const char *usageID)
 {
-virSecretObjPtr ret = NULL;
+virSecretObjPtr obj = NULL;
 struct virSecretSearchData data = { .usageType = usageType,
 .usageID = usageID };
 
-ret = virHashSearch(secrets->objs, virSecretObjSearchName, );
-if (ret)
-virObjectRef(ret);
-return ret;
+obj = virHashSearch(secrets->objs, virSecretObjSearchName, );
+if (obj)
+virObjectRef(obj);
+return obj;
 }
 
 
@@ -253,14 +253,14 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
 int usageType,
 const char *usageID)
 {
-virSecretObjPtr ret;
+virSecretObjPtr obj;
 
 virObjectLock(secrets);
-ret = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
+obj = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
 virObjectUnlock(secrets);
-if (ret)
-virObjectLock(ret);
-return ret;
+if 

[libvirt] [PATCH 01/14] secret: Need to set data->error on VIR_ALLOC_N failure

[John Ferlan]

Commit id 'bb1fba629' neglected to set when creating the function.

Signed-off-by: John Ferlan 
---
 src/conf/virsecretobj.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 049cab3..cc18459 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -457,8 +457,10 @@ virSecretObjListGetHelper(void *payload,
 if (data->uuids) {
 char *uuidstr;
 
-if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0)
+if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) {
+data->error = true;
 goto cleanup;
+}
 
 virUUIDFormat(obj->def->uuid, uuidstr);
 data->uuids[data->got] = uuidstr;
-- 
2.9.3

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

[libvirt] [PATCH 00/14] Multiple cleanups within secretobj and secret_driver

[John Ferlan]

More adjustments in preparation for having virobject code handle the bulk
of the object management code.

I know it's a lot of patches, but most should be relatively simple - especially
the first 10... The last 4 set up for a "common" way to handle the config
directory space as well and could be a bit more controversial. Perhaps the
most being patch 12 which attempts to perform the mkdir of the configDir
during initialization rather than first "define" of secret. Although someone
could also pick out their own most favorite/controversial and surprise me.


John Ferlan (14):
  secret: Need to set data->error on VIR_ALLOC_N failure
  secret: Convert virsecretobjs.h to use "newer" formatting style
  secret: Make some virSecretObj* functions static
  secret: Have virSecretObjNew return locked object
  secret: Use consistent naming for variables
  secret: Use virSecretDefPtr rather than deref from virSecretObjPtr
  secret: Move virSecretObjListGetUUIDs
  secret: Change variable names for list traversals
  secret: Split apart NumOfSecrets and GetUUIDs callback function
  secret: Combine virSecretObjListAdd with Locked function
  secret: Alter FindByUUID to expect the formatted uuidstr
  secret: Generate configDir during driver initialization
  secret: Alter configFile/base64File mgmt
  secret: Clean up virSecretObjListExport logic

 src/conf/virsecretobj.c| 557 +++--
 src/conf/virsecretobj.h| 120 +-
 src/secret/secret_driver.c | 178 ---
 3 files changed, 442 insertions(+), 413 deletions(-)

-- 
2.9.3

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

[libvirt] [PATCH] test: Remove unnecessary unlocks in cleanup paths

[John Ferlan]

Commit id '865f479da' altered the logic to use a common test*ObjFindByName
helpers which would lock/unlock the test driver; however, a few cleanup paths
in that cleanup missed removing the Unlock, so remove it now.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 00999b1..2db3f7d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3931,7 +3931,6 @@ testInterfaceUndefine(virInterfacePtr iface)
 ret = 0;
 
  cleanup:
-testDriverUnlock(privconn);
 return ret;
 }
 
@@ -3960,7 +3959,6 @@ testInterfaceCreate(virInterfacePtr iface,
  cleanup:
 if (privinterface)
 virInterfaceObjUnlock(privinterface);
-testDriverUnlock(privconn);
 return ret;
 }
 
@@ -3989,7 +3987,6 @@ testInterfaceDestroy(virInterfacePtr iface,
  cleanup:
 if (privinterface)
 virInterfaceObjUnlock(privinterface);
-testDriverUnlock(privconn);
 return ret;
 }
 
@@ -4480,7 +4477,6 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
 if (privpool)
 virStoragePoolObjUnlock(privpool);
 testObjectEventQueue(privconn, event);
-testDriverUnlock(privconn);
 return ret;
 }
 
@@ -4591,7 +4587,6 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
 testObjectEventQueue(privconn, event);
 if (privpool)
 virStoragePoolObjUnlock(privpool);
-testDriverUnlock(privconn);
 return ret;
 }
 
-- 
2.9.3

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

Re: [libvirt] [PATCH v2 09/10] docs: Provide a nodedev driver stub documentation

[Pavel Hrdina]

On Mon, Apr 24, 2017 at 04:04:01PM +0200, Pavel Hrdina wrote:
> On Thu, Apr 20, 2017 at 03:05:59PM +0200, Erik Skultety wrote:
> > There's lot more to document about the nodedev driver, besides PCI and
> > SR-IOV (even this might need to be extended), but let's start small-ish
> > and at least have a page for it linked from the drivers.html.
> > 
> > Signed-off-by: Erik Skultety 
> > ---
> >  docs/drivers.html.in|   6 +-
> >  docs/drvnodedev.html.in | 184 
> > 
> >  2 files changed, 189 insertions(+), 1 deletion(-)
> >  create mode 100644 docs/drvnodedev.html.in
> > 
> > diff --git a/docs/drivers.html.in b/docs/drivers.html.in
> > index be7483b9b..61993861e 100644
> > --- a/docs/drivers.html.in
> > +++ b/docs/drivers.html.in
> > @@ -4,7 +4,11 @@
> >
> >  Internal drivers
> >  
> > -
> > +
> > +  Hypervisor drivers
> > +  Storage drivers
> > +  Node device driver
> > +
> >  
> >  
> >The libvirt public API delegates its implementation to one or
> > diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in
> > new file mode 100644
> > index 0..ed185c3df
> > --- /dev/null
> > +++ b/docs/drvnodedev.html.in
> > @@ -0,0 +1,184 @@
> > +
> > + > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;>
> > +http://www.w3.org/1999/xhtml;>
> > +  
> > +Host device management
> > +
> > +
> > +  Libvirt provides management of both physical and virtual host devices
> > +  (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, MDEV, DRM, etc. 
> > +  
> 
> You should use  and  instead of double  if we are using 
> paragraphs.
> 
> > +  The node device driver provides means to list and show details about 
> > host
> > +  devices (virsh nodedev-list,
> > +  virsh nodedev-dumpxml), which are generic and can be 
> > used
> > +  with all devices. It also provides means to create and destroy 
> > devices
> > +  (virsh nodedev-create, virsh 
> > nodedev-destroy)
> > +  which are meant to be used to create virtual devices, currently only
> > +  supported by NPIV
> > +  (http://wiki.libvirt.org/page/NPIV_in_libvirt;>more info 
> > about NPIV)). 
> > +  
> 
> Same here.
> 
> > +  Devices on the host system are arranged in a tree-like hierarchy, 
> > with
> > +  the root node being called computer. The node device 
> > driver
> > +  supports two backends to manage the devices, HAL and udev, with the 
> > former
> > +  being deprecated in favour of the latter.
> 
> Either remove the single  or replace it with a pair of  and  to 
> end
> current paragraph and start a new one.
> 
> > +  The generic format of a host device XML can be seen below.
> > +  To identify a device both within the host and the device tree 
> > hierarchy,
> > +  the following elements are used:
> > +
> > +  
> > +name
> > +
> > +  The device's name will be generated by libvirt using the 
> > subsystem,
> > +  like pci and the device's sysfs basename.
> > +
> > +path
> > +
> > +  Fully qualified sysfs path to the device.
> > +
> > +parent
> > +
> > +  This element identifies the parent node in the device hierarchy. 
> > The
> > +  value of the element will correspond with the device parent's
> > +  name element or computer if the device 
> > does
> > +  not have any parent.
> > +
> > +driver
> > +
> > +  This elements reports the driver in use for this device. The 
> > presence
> > +  of this element in the output XML depends on whether the 
> > underlying
> > +  device manager (most likely udev) exposes information about the
> > +  driver.
> > +
> > +capability
> > +
> > +  Describes the device in terms of feature support. The element 
> > has one
> > +  mandatory attribute type the value of which 
> > determines
> > +  the type of the device. Currently recognized values for the 
> > attribute
> > +  are:
> > +  system,
> > +  pci,
> > +  usb,
> > +  usb_device,
> > +  net,
> > +  scsi,
> > +  scsi_host (Since 0.4.7),
> > +  fc_host,
> > +  vports,
> > +  scsi_target (Since 
> > 0.7.3),
> > +  storage (Since 1.0.4),
> > +  scsi_generic (Since 
> > 1.0.7),
> > +  drm (Since 3.1.0), and
> > +  mdev (Since 3.2.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 

Re: [libvirt] [PATCH v2 09/10] docs: Provide a nodedev driver stub documentation

[Pavel Hrdina]

On Thu, Apr 20, 2017 at 03:05:59PM +0200, Erik Skultety wrote:
> There's lot more to document about the nodedev driver, besides PCI and
> SR-IOV (even this might need to be extended), but let's start small-ish
> and at least have a page for it linked from the drivers.html.
> 
> Signed-off-by: Erik Skultety 
> ---
>  docs/drivers.html.in|   6 +-
>  docs/drvnodedev.html.in | 184 
> 
>  2 files changed, 189 insertions(+), 1 deletion(-)
>  create mode 100644 docs/drvnodedev.html.in
> 
> diff --git a/docs/drivers.html.in b/docs/drivers.html.in
> index be7483b9b..61993861e 100644
> --- a/docs/drivers.html.in
> +++ b/docs/drivers.html.in
> @@ -4,7 +4,11 @@
>
>  Internal drivers
>  
> -
> +
> +  Hypervisor drivers
> +  Storage drivers
> +  Node device driver
> +
>  
>  
>The libvirt public API delegates its implementation to one or
> diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in
> new file mode 100644
> index 0..ed185c3df
> --- /dev/null
> +++ b/docs/drvnodedev.html.in
> @@ -0,0 +1,184 @@
> +
> + "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;>
> +http://www.w3.org/1999/xhtml;>
> +  
> +Host device management
> +
> +
> +  Libvirt provides management of both physical and virtual host devices
> +  (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, MDEV, DRM, etc. 
> +  

You should use  and  instead of double  if we are using paragraphs.

> +  The node device driver provides means to list and show details about 
> host
> +  devices (virsh nodedev-list,
> +  virsh nodedev-dumpxml), which are generic and can be used
> +  with all devices. It also provides means to create and destroy devices
> +  (virsh nodedev-create, virsh nodedev-destroy)
> +  which are meant to be used to create virtual devices, currently only
> +  supported by NPIV
> +  (http://wiki.libvirt.org/page/NPIV_in_libvirt;>more info 
> about NPIV)). 
> +  

Same here.

> +  Devices on the host system are arranged in a tree-like hierarchy, with
> +  the root node being called computer. The node device 
> driver
> +  supports two backends to manage the devices, HAL and udev, with the 
> former
> +  being deprecated in favour of the latter.

Either remove the single  or replace it with a pair of  and  to end
current paragraph and start a new one.

> +  The generic format of a host device XML can be seen below.
> +  To identify a device both within the host and the device tree 
> hierarchy,
> +  the following elements are used:
> +
> +  
> +name
> +
> +  The device's name will be generated by libvirt using the subsystem,
> +  like pci and the device's sysfs basename.
> +
> +path
> +
> +  Fully qualified sysfs path to the device.
> +
> +parent
> +
> +  This element identifies the parent node in the device hierarchy. 
> The
> +  value of the element will correspond with the device parent's
> +  name element or computer if the device 
> does
> +  not have any parent.
> +
> +driver
> +
> +  This elements reports the driver in use for this device. The 
> presence
> +  of this element in the output XML depends on whether the underlying
> +  device manager (most likely udev) exposes information about the
> +  driver.
> +
> +capability
> +
> +  Describes the device in terms of feature support. The element has 
> one
> +  mandatory attribute type the value of which determines
> +  the type of the device. Currently recognized values for the 
> attribute
> +  are:
> +  system,
> +  pci,
> +  usb,
> +  usb_device,
> +  net,
> +  scsi,
> +  scsi_host (Since 0.4.7),
> +  fc_host,
> +  vports,
> +  scsi_target (Since 0.7.3),
> +  storage (Since 1.0.4),
> +  scsi_generic (Since 1.0.7),
> +  drm (Since 3.1.0), and
> +  mdev (Since 3.2.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.
> +
> +  
> +
> +Basic structure of a node device
> +
> +device
> +  namepci__00_17_0/name
> +  path/sys/devices/pci:00/:00:17.0/path
> +  parentcomputer/parent
> +  driver
> +nameahci/name
> +  /driver
> +  capability type='pci'
> +...
> +  /capability
> +/device
> +
> +
> +
> +PCI host devices
> +
> 

Re: [libvirt] error in docs https://libvirt.org/html/libvirt-libvirt-domain.html

[Michal Privoznik]

On 04/24/2017 02:34 PM, Vasiliy Tolstov wrote:

2017-04-24 15:30 GMT+03:00 Michal Privoznik :

Diggin' deeper, this is problem of apibuild.py script which also generates 
docs/libvirt-api.xml. So our bindings have incorrect description for the values 
too. I am looking into this now.



Thanks!



Actually, after seeing that python code, I give up. If somebody wants to 
fix it, please do.


Michal

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

Re: [libvirt] [PATCH v2] autogen.sh: Improve and generalize

[Daniel P. Berrange]

On Thu, Apr 20, 2017 at 07:54:39PM +0200, Andrea Bolognani wrote:
> The goal is twofold: firstly, we want to extend the script so
> that it can deal with more than a single git submodule, and
> secondly we'd like to reduce the amount of duplicated code.
> Moreover, since we're making heavy changes to the code anyway,
> we might as well make sure it follows a somewhat consistent
> coding style too.
> 
> To reduce code duplication, we introduce a new --dry-run
> option, which can be used by third parties to figure out
> whether calling autogen.sh is necessary or not: this allows
> us to get rid of the reimplementation of part of the logic
> in cfg.mk and guarantee they'll never get out of sync.
> 
> Other changes include: making dirty submodules checking and
> cleaning entirely independent of other operations; removing
> the use of 'set -e' and handling errors explicitly instead;
> better parsing of command line arguments.
> ---
> Changes from [v1]:
> 
>   * drop the generic submodule update framework and simply
> update all submodules unconditionally, *then* perform
> any required gnulib-specific step;
> 
>   * handle "autogen not required" and "dry run error"
> differently in cfg.mk;
> 
>   * improve comments.
> 
> [v1] https://www.redhat.com/archives/libvir-list/2017-April/msg00816.html
> 
>  autogen.sh | 253 
> -
>  cfg.mk |  53 ++---
>  2 files changed, 193 insertions(+), 113 deletions(-)

I applied this to my local git master & rebased my keycodemap code on
to it.

I then did a clean build of master, then switched to my keycodemap
branch again, and tried make. It correctly updated the new git submodule
and avoided re-running gnulib bootstrap.

So this looks to be functioning as desired.

ACK



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 v2 07/10] nodedev: Introduce the mdev capability to a PCI parent device

[Pavel Hrdina]

On Thu, Apr 20, 2017 at 03:05:57PM +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' with the resulting XML of the following format:
> 
> 
> ...
>   
>   ...
> 
>   
> optional, raw, unstructured resource allocation data
> 
> vfio-pci
> NUM
>   
>   ...
>   
>   ...
>   
> 
>   
> ...
> 

You've mentioned it in the cover letter and asked me privately to share
my opinion about the mdev capability.  I agree that we should split the
capability into one for parent device and one for child device.  Having
one capability for two different devices where the capability means
something else based on the device doesn't seem to be a good idea especially
if we have an API that allows you list devices whit some specific capability.

The internal representation of the capability is one structure where both
parent and child shares only *type*.  The suggested names mdev-parent and
mdev-child seems as good names if the "parent" and "child" naming is generally
used for mdev devices.

> 
> Signed-off-by: Erik Skultety 
> ---
>  docs/schemas/nodedev.rng  |  24 
>  src/conf/node_device_conf.c   | 103 +
>  src/conf/node_device_conf.h   |  17 +++
>  src/libvirt_private.syms  |   1 +
>  src/node_device/node_device_udev.c| 133 
> ++
>  tests/nodedevschemadata/pci__02_10_7_mdev.xml |  27 +
>  6 files changed, 305 insertions(+)
>  create mode 100644 tests/nodedevschemadata/pci__02_10_7_mdev.xml
> 
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 0f90a73c8..4b5dca777 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -205,6 +205,30 @@
>  
>  
>  
> +  
> +
> +  mdev
> +
> +
> +  
> +
> +  
> +  
> +
> +  
> +  
> +
> +  vfio-pci
> +
> +  
> +  
> +
> +  
> +
> +  
> +   
> +
> +
>
>  
>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index fdddc97eb..fe4f1bc60 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -87,6 +87,26 @@ virNodeDevCapsDefParseString(const char *xpath,
>  }
>  
>  
> +static void
> +virNodeDevCapMdevClear(virNodeDevCapMdevPtr mdev)
> +{
> +VIR_FREE(mdev->type);
> +VIR_FREE(mdev->name);
> +VIR_FREE(mdev->device_api);
> +}

This won't be required if we split the mdev capability.

> +void
> +virNodeDevCapMdevFree(virNodeDevCapMdevPtr mdev)
> +{
> +if (!mdev)
> +return;
> +
> +virNodeDevCapMdevClear(mdev);
> +VIR_FREE(mdev);
> +}
> +
> +
>  void
>  virNodeDeviceDefFree(virNodeDeviceDefPtr def)
>  {
> @@ -264,6 +284,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.nmdevs; i++) {
> +virNodeDevCapMdevPtr mdev = data->pci_dev.mdevs[i];
> +virBufferEscapeString(buf, "\n", mdev->type);
> +virBufferAdjustIndent(buf, 2);
> +if (mdev->name)
> +virBufferAsprintf(buf, "%s\n",
> +  mdev->name);

This will fit on one line.

> +virBufferAsprintf(buf, "%s\n",
> +  mdev->device_api);
> +virBufferAsprintf(buf,
> +  
> "%u\n",
> +  mdev->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);
> @@ -1358,6 +1399,62 @@ virNodeDevPCICapSRIOVParseXML(xmlXPathContextPtr ctxt,
>  
>  
>  static int
> +virNodeDevPCICapMediatedDevParseXML(xmlXPathContextPtr ctxt,
> +virNodeDevCapPCIDevPtr pci_dev)
> +{
> +int ret = -1;
> +xmlNodePtr orignode = NULL;
> +xmlNodePtr *nodes = NULL;
> +int nmdevs = virXPathNodeSet("./type", ctxt, );
> +virNodeDevCapMdevPtr mdev = NULL;
> +size_t i;
> +
> +orignode = ctxt->node;
> +for (i = 0; i < nmdevs; i++) {
> +  

[libvirt] [PATCH v4 3/3] xlconfigtest: add tests for 'nestedhvm' support

[Wim Ten Have]

From: Wim ten Have 

Testing various configuration schemas targeting postive and negative
nestedhvm under libvirt  configuration.

Mode "host-passthrough" generates nestedhvm=1 in/from xl format where

Intel virtualization (VT-x):


or

AMD virtualization (AMD-V):


disables virtualization mode under guest domains.

Signed-off-by: Wim ten Have 
Signed-off-by: Jim Fehlig 
---
 tests/testutilsxen.c   | 30 +++
 .../test-fullvirt-nestedhvm-disabled.cfg   | 26 +
 .../test-fullvirt-nestedhvm-disabled.xml   | 61 ++
 tests/xlconfigdata/test-fullvirt-nestedhvm.cfg | 26 +
 tests/xlconfigdata/test-fullvirt-nestedhvm.xml | 59 +
 tests/xlconfigtest.c   |  2 +
 6 files changed, 204 insertions(+)
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.cfg
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.xml

diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index 122789c..c4fc146 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -6,6 +6,33 @@
 #include "testutilsxen.h"
 #include "domain_conf.h"
 
+static virCPUFeatureDef cpuDefaultFeatures[] = {
+{ (char *) "ds",-1 },
+{ (char *) "acpi",  -1 },
+{ (char *) "ss",-1 },
+{ (char *) "ht",-1 },
+{ (char *) "tm",-1 },
+{ (char *) "pbe",   -1 },
+{ (char *) "ds_cpl",-1 },
+{ (char *) "vmx",   -1 },
+{ (char *) "est",   -1 },
+{ (char *) "tm2",   -1 },
+{ (char *) "cx16",  -1 },
+{ (char *) "xtpr",  -1 },
+{ (char *) "lahf_lm",   -1 },
+};
+static virCPUDef cpuDefaultData = {
+.type = VIR_CPU_TYPE_HOST,
+.arch = VIR_ARCH_X86_64,
+.model = (char *) "core2duo",
+.vendor = (char *) "Intel",
+.sockets = 1,
+.cores = 2,
+.threads = 1,
+.nfeatures = ARRAY_CARDINALITY(cpuDefaultFeatures),
+.nfeatures_max = ARRAY_CARDINALITY(cpuDefaultFeatures),
+.features = cpuDefaultFeatures,
+};
 
 virCapsPtr testXenCapsInit(void)
 {
@@ -88,6 +115,9 @@ testXLInitCaps(void)
 if ((caps = virCapabilitiesNew(virArchFromHost(),
false, false)) == NULL)
 return NULL;
+
+caps->host.cpu = virCPUDefCopy();
+
 nmachines = ARRAY_CARDINALITY(x86_machines);
 if ((machines = virCapabilitiesAllocMachines(x86_machines, nmachines)) == 
NULL)
 goto cleanup;
diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg 
b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg
new file mode 100644
index 000..d4b9f45
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg
@@ -0,0 +1,26 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+pae = 1
+acpi = 1
+apic = 1
+viridian = 0
+rtc_timeoffset = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-system-i386"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ]
+parallel = "none"
+serial = "none"
+builder = "hvm"
+boot = "d"
+nestedhvm = 0
+disk = [ 
"format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", 
"format=qcow2,vdev=hdb,access=rw,backendtype=qdisk,target=/var/lib/libvirt/images/XenGuest2-home",
 
"format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso"
 ]
diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml 
b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml
new file mode 100644
index 000..58b6338
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml
@@ -0,0 +1,61 @@
+
+  XenGuest2
+  c7a5fdb2-cdaf-9455-926a-d65c16db1809
+  592896
+  403456
+  1
+  
+hvm
+/usr/lib/xen/boot/hvmloader
+
+  
+  
+
+
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/lib/xen/bin/qemu-system-i386
+
+  
+  
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+  
+  
+  
+  
+
+
+
+  
+  
+  
+  
+
+
+
+
+  
+
+
+  
+
+  
+
diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm.cfg 
b/tests/xlconfigdata/test-fullvirt-nestedhvm.cfg
new file mode 100644
index 000..281f126
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-nestedhvm.cfg
@@ -0,0 +1,26 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+pae = 1
+acpi = 1
+apic = 1
+viridian = 0
+rtc_timeoffset = 0
+localtime = 0
+on_poweroff = "destroy"

[libvirt] [PATCH v4 0/3] libxl: nestedhvm support

[Wim Ten Have]

From: Wim ten Have 

This patch enhances host-passthrough capability to advertise the
required vendor CPU virtualization feature which will be used to
enable 'nestedhvm' in the libxl driver.

Wim ten Have (3):
  libxl: set nestedhvm for mode host-passthrough
  xenconfig: add conversions for xen-xl
  xlconfigtest: add tests for 'nestedhvm' support

 cfg.mk |  4 +-
 src/libxl/libxl_conf.c | 39 ++-
 src/libxl/libxl_conf.h |  1 +
 src/libxl/libxl_domain.c   |  2 +-
 src/xenconfig/xen_xl.c | 76 ++
 tests/testutilsxen.c   | 30 +
 .../test-fullvirt-nestedhvm-disabled.cfg   | 26 
 .../test-fullvirt-nestedhvm-disabled.xml   | 61 +
 tests/xlconfigdata/test-fullvirt-nestedhvm.cfg | 26 
 tests/xlconfigdata/test-fullvirt-nestedhvm.xml | 59 +
 tests/xlconfigtest.c   |  2 +
 11 files changed, 322 insertions(+), 4 deletions(-)
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.cfg
 create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.xml

-- 
2.9.3

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

[libvirt] [PATCH v4 1/3] libxl: set nestedhvm for mode host-passthrough

[Wim Ten Have]

From: Wim ten Have 

Xen feature nestedhvm is the option on Xen 4.4+ which enables
nested virtualization when mode host-passthrough is applied.

nested HVM is enabled by adding below on the target domain;


Virtualization on target domain can be disabled by specifying
such under feature policy rule on target name;

[On Intel (VT-x) architecture]


or:

[On AMD (AMD-V) architecture]


Signed-off-by: Joao Martins 
Signed-off-by: Wim ten Have 
---
 src/libxl/libxl_conf.c   | 39 ++-
 src/libxl/libxl_conf.h   |  1 +
 src/libxl/libxl_domain.c |  2 +-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4bab651..56bc097 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -47,6 +47,7 @@
 #include "libxl_utils.h"
 #include "virstoragefile.h"
 #include "secret_util.h"
+#include "cpu/cpu.h"
 
 
 #define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -293,6 +294,7 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
 static int
 libxlMakeDomBuildInfo(virDomainDefPtr def,
   libxl_ctx *ctx,
+  virCapsPtr caps,
   libxl_domain_config *d_config)
 {
 libxl_domain_build_info *b_info = _config->b_info;
@@ -374,6 +376,40 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
   def->features[VIR_DOMAIN_FEATURE_ACPI] ==
   VIR_TRISTATE_SWITCH_ON);
 
+if (caps &&
+def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+bool hasHwVirt = false;
+bool svm = false, vmx = false;
+
+if (ARCH_IS_X86(def->os.arch)) {
+vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, 
"vmx");
+svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu, 
"svm");
+hasHwVirt = vmx | svm;
+}
+
+if (def->cpu->nfeatures) {
+for (i = 0; i < def->cpu->nfeatures; i++) {
+
+switch (def->cpu->features[i].policy) {
+
+case VIR_CPU_FEATURE_DISABLE:
+case VIR_CPU_FEATURE_FORBID:
+if ((vmx && STREQ(def->cpu->features[i].name, 
"vmx")) ||
+(svm && STREQ(def->cpu->features[i].name, 
"svm")))
+hasHwVirt = false;
+break;
+
+case VIR_CPU_FEATURE_FORCE:
+case VIR_CPU_FEATURE_REQUIRE:
+case VIR_CPU_FEATURE_OPTIONAL:
+case VIR_CPU_FEATURE_LAST:
+break;
+}
+}
+}
+libxl_defbool_set(_info->u.hvm.nested_hvm, hasHwVirt);
+}
+
 if (def->nsounds > 0) {
 /*
  * Use first sound device.  man xl.cfg(5) describes soundhw as
@@ -2089,6 +2125,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
const char *channelDir LIBXL_ATTR_UNUSED,
libxl_ctx *ctx,
+   virCapsPtr caps,
libxl_domain_config *d_config)
 {
 libxl_domain_config_init(d_config);
@@ -2096,7 +2133,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
 if (libxlMakeDomCreateInfo(ctx, def, _config->c_info) < 0)
 return -1;
 
-if (libxlMakeDomBuildInfo(def, ctx, d_config) < 0)
+if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0)
 return -1;
 
 if (libxlMakeDiskList(def, d_config) < 0)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index c653c9f..264df11 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -217,6 +217,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
const char *channelDir LIBXL_ATTR_UNUSED,
libxl_ctx *ctx,
+   virCapsPtr caps,
libxl_domain_config *d_config);
 
 static inline void
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ea28c93..256cf1d 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1256,7 +1256,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
 goto cleanup_dom;
 
 if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def,
-   cfg->channelDir, cfg->ctx, _config) < 0)
+   cfg->channelDir, cfg->ctx, cfg->caps, 
_config) < 0)
 goto cleanup_dom;
 
 if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, _config) < 0)
-- 
2.9.3

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

[libvirt] [PATCH v4 2/3] xenconfig: add conversions for xen-xl

[Wim Ten Have]

From: Wim ten Have 

Per xen-xl conversions from and to native under host-passthrough
mode we take care for Xen (nestedhvm = mode) applied and inherited
settings generating or processing correct feature policy:

[On Intel (VT-x) architectures]


or

[On AMD (AMD-V) architectures]


It will then generate (or parse) for nestedhvm=1 in/from xl format.

Signed-off-by: Joao Martins 
Signed-off-by: Wim ten Have 
---
 cfg.mk |  4 +--
 src/xenconfig/xen_xl.c | 76 ++
 2 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 09548fe..3da8332 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -768,7 +768,7 @@ sc_prohibit_gettext_markup:
 # lower-level code must not include higher-level headers.
 cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.))
 cross_dirs_re=($(subst / ,/|,$(cross_dirs)))
-mid_dirs=access|conf|cpu|locking|logging|network|node_device|rpc|security|storage
+mid_dirs=access|conf|cpu|locking|logging|network|node_device|rpc|security|storage|xen
 sc_prohibit_cross_inclusion:
@for dir in $(cross_dirs); do   \
  case $$dir in \
@@ -777,7 +777,7 @@ sc_prohibit_cross_inclusion:
locking/) safe="($$dir|util|conf|rpc)";;\
cpu/| network/| node_device/| rpc/| security/| storage/)\
  safe="($$dir|util|conf|storage)";;\
-   xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen)";;   \
+   xenapi/) safe="($$dir|util|conf|xen)";; \
*) safe="($$dir|$(mid_dirs)|util)";;\
  esac; \
  in_vc_files="^src/$$dir"  \
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index a5b342a..4f24d45 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -34,6 +34,7 @@
 #include "virstoragefile.h"
 #include "xen_xl.h"
 #include "libxl_capabilities.h"
+#include "cpu/cpu.h"
 
 #define VIR_FROM_THIS VIR_FROM_XENXL
 
@@ -106,6 +107,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, 
virCapsPtr caps)
 if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
 const char *bios;
 const char *boot;
+int val = 0;
 
 if (xenConfigGetString(conf, "bios", , NULL) < 0)
 return -1;
@@ -164,6 +166,52 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, 
virCapsPtr caps)
 }
 def->os.nBootDevs++;
 }
+
+if (xenConfigGetBool(conf, "nestedhvm", , -1) < 0)
+return -1;
+
+if (val == 1) {
+virCPUDefPtr cpu;
+
+if (VIR_ALLOC(cpu) < 0)
+return -1;
+
+cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+cpu->type = VIR_CPU_TYPE_GUEST;
+def->cpu = cpu;
+} else if (val == 0) {
+const char *vtfeature = NULL;
+
+if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch)) {
+if (virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"))
+vtfeature = "vmx";
+else if (virCPUCheckFeature(caps->host.arch, caps->host.cpu, 
"svm"))
+vtfeature = "svm";
+}
+
+if (vtfeature) {
+virCPUDefPtr cpu;
+
+if (VIR_ALLOC(cpu) < 0)
+return -1;
+
+if (VIR_ALLOC(cpu->features) < 0) {
+VIR_FREE(cpu);
+return -1;
+}
+
+if (VIR_STRDUP(cpu->features->name, vtfeature) < 0) {
+VIR_FREE(cpu->features);
+VIR_FREE(cpu);
+return -1;
+}
+cpu->features->policy = VIR_CPU_FEATURE_DISABLE;
+cpu->nfeatures = cpu->nfeatures_max = 1;
+cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+cpu->type = VIR_CPU_TYPE_GUEST;
+def->cpu = cpu;
+}
+}
 } else {
 if (xenConfigCopyStringOpt(conf, "bootloader", >os.bootloader) < 
0)
 return -1;
@@ -899,6 +947,34 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
 if (xenConfigSetString(conf, "boot", boot) < 0)
 return -1;
 
+if (def->cpu &&
+def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
+bool hasHwVirt = true;
+
+if (def->cpu->nfeatures) {
+for (i = 0; i < def->cpu->nfeatures; i++) {
+
+switch (def->cpu->features[i].policy) {
+case VIR_CPU_FEATURE_DISABLE:
+case VIR_CPU_FEATURE_FORBID:
+if (STREQ(def->cpu->features[i].name, "vmx") ||
+

Re: [libvirt] error in docs https://libvirt.org/html/libvirt-libvirt-domain.html

[Vasiliy Tolstov]

2017-04-24 15:30 GMT+03:00 Michal Privoznik :
> Diggin' deeper, this is problem of apibuild.py script which also generates 
> docs/libvirt-api.xml. So our bindings have incorrect description for the 
> values too. I am looking into this now.


Thanks!

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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

Re: [libvirt] [PATCH] util: relax virNetDevSetCoalesce() stub

[Roman Bogorodskiy]

  Martin Kletzander wrote:

> On Sun, Apr 23, 2017 at 06:57:07PM +0400, Roman Bogorodskiy wrote:
> >Currently, virNetDevSetCoalesce() stub is always returning error. As
> >it's used by virNetDevTapCreateInBridgePort(), it essentially breaks
> >bridged networking if coalesce is not supported.
> >
> >To make it work, relax the stub to trigger error only when its
> >coalesce argument is not NULL, otherwise report success.
> >---
> > src/util/virnetdev.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> 
> That should be fixed in the other stub as well.  ACK with that added.

Adjusted the other stub and pushed, thanks!

Roman Bogorodskiy


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

Re: [libvirt] error in docs https://libvirt.org/html/libvirt-libvirt-domain.html

[Michal Privoznik]

On 04/18/2017 03:52 PM, Vasiliy Tolstov wrote:
> virDomainMigrateFlags have errors
> VIR_MIGRATE_LIVE have invalid comment and as i see all comments are
> shifted down by one point.
> 

It's not just that. Basically anything that is the following form:

typedef enum {
  /* description to value1 */
  value1,

  /* description to value2 */
  value2,
};

The other example is:

https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMemoryStatTags

Diggin' deeper, this is problem of apibuild.py script which also generates 
docs/libvirt-api.xml. So our bindings have incorrect description for the values 
too. I am looking into this now.

Michal

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

Re: [libvirt] [PATCH] Fix minor typos

[Erik Skultety]

On Sat, Apr 22, 2017 at 10:11:01PM +0300, Yuri Chornoivan wrote:
> Hi,
>
> Attached is a minor fix for the typos in libvirt's git/master.
>
> Best regards,
> Yuri

ACK, I'll push the patch in a moment.

Erik

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

Re: [libvirt] [PATCH go-xml] add support for domain features

[Daniel P. Berrange]

On Sat, Apr 22, 2017 at 02:53:48PM -0700, Ryan Goodfellow wrote:
> This commit adds support for domain features. It does so by introducing
> a new family of types DomainFeature*. The aggregate type
> DomainFeatureList has been added to the Domain type to plumb in the new
> type family. Testing has also been added in domain_test.go
> ---
>  domain.go  | 91 
> +++---
>  domain_test.go | 55 +++
>  2 files changed, 130 insertions(+), 16 deletions(-)
> 
> diff --git a/domain.go b/domain.go
> index cccd9a6..c9ffaef 100644
> --- a/domain.go
> +++ b/domain.go
> @@ -371,23 +371,82 @@ type DomainCPU struct {
>   Features []DomainCPUFeature `xml:"feature"`
>  }
>  
> +type DomainFeature struct {
> + State string `xml:"state,attr,omitempty"`
> +}

There is no 'state' attribute on most top level features - this is just
something used under the hyperv features

> +
> +type DomainFeatureAPIC struct {
> + DomainFeature
> + EOI string `xml:"eio,attr,omitempty"`
> +}

So this is wrong for example

> +
> +type DomainFeatureVendorId struct {
> + DomainFeature
> + Value string `xml:"value,attr,omitempty"`
> +}
> +
> +type DomainFeatureSpinlocks struct {
> + DomainFeature
> + Retries uint `xml:"retries,attr,omitempty"`
> +}

Since these are hyperv featurs, the struct name should have
HyperV in the name too.

> +
> +type DomainFeatureHyperV struct {
> + DomainFeature
> + Relaxed   *DomainFeature  `xml:"relaxed,omitempty"`
> + VAPIC *DomainFeature  `xml:"vapic,omitempty"`
> + Spinlocks *DomainFeatureSpinlocks `xml:"spinlocks,omitempty"`
> + VPIndex   *DomainFeature  `xml:"vpindex,omitempty"`
> + Runtime   *DomainFeature  `xml:"runtime,omitempty"`
> + Synic *DomainFeature  `xml:"synic,omitempty"`
> + STimer*DomainFeature  `xml:"stimer,omitempty"`
> + Reset *DomainFeature  `xml:"reset,omitempty"`
> + VendorId  *DomainFeatureVendorId  `xml:"vendor_id,omitempty"`

There's no need to use 'omitempty' when the type is a struct
pointer - its only needed for scalar types.

> +}
> +
> +type DomainFeatureKVM struct {
> + DomainFeature
> + Hidden *DomainFeature `xml:"hidden,omitempty"`
> +}
> +
> +type DomainFeatureGIC struct {
> + DomainFeature
> + Version string `xml:"version,attr,omitempty"`
> +}
> +
> +type DomainFeatureList struct {
> + PAE*DomainFeature   `xml:"pae,omitempty"`
> + ACPI   *DomainFeature   `xml:"acpi,omitempty"`
> + APIC   *DomainFeatureAPIC   `xml:"apic,omitempty"`
> + HAP*DomainFeature   `xml:"hap,omitempty"`
> + Viridian   *DomainFeature   `xml:"viridian,omitempty"`
> + PrivNet*DomainFeature   `xml:"privnet,omitempty"`
> + HyperV *DomainFeatureHyperV `xml:"hyperv,omitempty"`
> + KVM*DomainFeatureKVM`xml:"kvm,omitempty"`
> + PVSpinlock *DomainFeature   `xml:"pvspinlock,omitempty"`
> + PMU*DomainFeature   `xml:"pmu,omitempty"`
> + VMPort *DomainFeature   `xml:"vmport,omitempty"`
> + GIC*DomainFeatureGIC`xml:"gic,omitempty"`
> + SMM*DomainFeature   `xml:"smm,omitempty"`
> +}

None of these should use 'DomainFeature', since they do not
want a 'state' attribute. Also same note about omitempty not
being needed.

> +
>  type Domain struct {
> - XMLName   xml.Name  `xml:"domain"`
> - Type  string`xml:"type,attr,omitempty"`
> - Name  string`xml:"name"`
> - UUID  string`xml:"uuid,omitempty"`
> - Memory*DomainMemory `xml:"memory"`
> - CurrentMemory *DomainMemory `xml:"currentMemory"`
> - MaximumMemory *DomainMaxMemory  `xml:"maxMemory"`
> - VCPU  *DomainVCPU   `xml:"vcpu"`
> - CPU   *DomainCPU`xml:"cpu"`
> - Resource  *DomainResource   `xml:"resource"`
> - Devices   *DomainDeviceList `xml:"devices"`
> - OS*DomainOS `xml:"os"`
> - SysInfo   *DomainSysInfo`xml:"sysinfo"`
> - OnPoweroffstring`xml:"on_poweroff,omitempty"`
> - OnReboot  string`xml:"on_reboot,omitempty"`
> - OnCrash   string`xml:"on_crash,omitempty"`
> + XMLName   xml.Name   `xml:"domain"`
> + Type  string `xml:"type,attr,omitempty"`
> + Name  string `xml:"name"`
> + UUID  string `xml:"uuid,omitempty"`
> + Memory*DomainMemory  `xml:"memory"`
> + CurrentMemory *DomainMemory  `xml:"currentMemory"`
> + MaximumMemory *DomainMaxMemory   `xml:"maxMemory"`
> + VCPU  *DomainVCPU`xml:"vcpu"`
> + CPU   *DomainCPU `xml:"cpu"`
> + Resource  

Re: [libvirt] [PATCH go-xml] Support for filesystem devices

[Daniel P. Berrange]

On Tue, Apr 18, 2017 at 03:01:18PM -0700, Ryan Goodfellow wrote:
> This commit adds filesystem device support. A new family of types
> DomainFilesystem* are introduced and plumbed into the DomainDeviceList
> struct.
> 
> Testing has also been included.
> ---
>  domain.go  | 40 
>  domain_test.go | 55 +++
>  2 files changed, 95 insertions(+)

Thanks, I've pushed this to master now.


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 v2 04/10] docs: Utilize our XSLT list generating template more

[Erik Skultety]

On Fri, Apr 21, 2017 at 02:51:50PM +0200, Pavel Hrdina wrote:
> On Thu, Apr 20, 2017 at 03:05:54PM +0200, Erik Skultety wrote:
> > Since we do have this template at hand, why not using it wherever
> > possible (list of supported pool types and remote access section).
> > Also, perform some stylistic micro adjustments.
> >
> > Signed-off-by: Erik Skultety 
> > ---
> >  docs/remote.html.in  | 106 
> > +++
> >  docs/storage.html.in |  62 ++
> >  2 files changed, 41 insertions(+), 127 deletions(-)
>
> ACK
>
> Pavel

I went ahead and pushed patches 1-4, as those are just cleanups.

Thanks,
Erik

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

Re: [libvirt] [PATCH v2 06/10] nodedev: Introduce the mdev capability to the nodedev driver structure

[Pavel Hrdina]

On Thu, Apr 20, 2017 at 03:05:56PM +0200, Erik Skultety wrote:
> Besides updating the capability enum, this patch introduces
> 'virNodeDevCapMdev' structure which will store everything libvirt can
> gather from sysfs about a mediated device. Since we need to report the
> info for both the mediated child device and its parent mdev capabilities
> (merely the mdev types' attributes), this structure serves in both of
> these cases with the difference being that the amount of data it holds
> depends on the specific scenario (child vs parent).

The commit message is no longer valid since you've moved the struct into
a different patch.

Pavel


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

Re: [libvirt] [PATCH] [PATCH] rpc: fix keep alive timer segfault

[Michal Privoznik]

On 04/18/2017 03:55 AM, Yi Wang wrote:

ka maybe have been freeed in virObjectUnref, application using
virKeepAliveTimer will segfault when unlock ka. We should keep
ka's refs positive before using it.

Signed-off-by: Yi Wang 
---
 src/rpc/virkeepalive.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)


So after discussion in the other thread [1], I've squashed in the 
backtrace you posted there, ACKed and pushed.


Congratulations on your first libvirt contribution!

Michal

1: https://www.redhat.com/archives/libvir-list/2017-April/msg01024.html

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

Re: [libvirt] [PATCH] [PATCH] rpc: fix keep alive timer segfault

[Daniel P. Berrange]

On Mon, Apr 24, 2017 at 11:15:31AM +0200, Michal Privoznik wrote:
> On 04/24/2017 05:31 AM, wang.y...@zte.com.cn wrote:
> > Hi Michal,
> > 
> > 
> > 
> > Thanks for your review. The problem occured in a python applicatin using 
> > libvirt-python, which has no ref().  If we unref() first in 
> > virKeepAliveTimer, we may get a segfault in virObjectUnlock() when cleanup, 
> > so I suppose that my patch is safer, :-) Here is the backtrace:
> > 
> > 
> 
> Okay, now it makes more sense. However, the ref() I was referring to is our
> libvirt virObjectRef() not the one in python. The code where segfault occurs
> is too deep for python to see anyway. One thing that I am still worried
> about is doing virObjectRef() before virObjectLock(). I think these two
> steps should be swapped.

That ordering should not matter. At the time this code runs, we *must*
be guaranteed that we already hold a valid ref on the object. If we
did not have that existing ref, then both virObjectRef and virObjectLock
could segv. So the 'virObjectRef' must be acquiring a second reference,
and thus doing lock first would not give us any further protection

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] [PATCH] rpc: fix keep alive timer segfault

[Michal Privoznik]

On 04/24/2017 05:31 AM, wang.y...@zte.com.cn wrote:

Hi Michal,



Thanks for your review. The problem occured in a python applicatin using 
libvirt-python, which has no ref().  If we unref() first in virKeepAliveTimer, 
we may get a segfault in virObjectUnlock() when cleanup, so I suppose that my 
patch is safer, :-) Here is the backtrace:




Okay, now it makes more sense. However, the ref() I was referring to is 
our libvirt virObjectRef() not the one in python. The code where 
segfault occurs is too deep for python to see anyway. One thing that I 
am still worried about is doing virObjectRef() before virObjectLock(). I 
think these two steps should be swapped.


Can you check please whether that also fix your problem when you switch 
those two lines?


Michal

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

Re: [libvirt] [PATCH] RFE: virsh: add domxml-to-native [--domain DOMAIN] option

[Peter Krempa]

On Sun, Apr 23, 2017 at 20:54:47 -0400, Dan wrote:

Please use your full name for patch submissions.

> Bug 835476 RFE: virsh: add domxml-to-native --domain option (for existing
> VM) [1]
> 
> virsh DOMAIN COMMAND domxml-to-native did not support domain (id|uuid|name)
> as input for generating hypervisor agent native command, instead only
> supported
> XML input from STDIN. Here in this patch, it supports the following syntax:
> domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it supports
> either designating domain (domain id, uuid, or name), or path to XML domain
> configuration file; NOTE that it deprecated existing STDIN input passing XML
> functionality. It was tested on the test mock driver and QEMU.
> 
> 
> 
> 
> [1]. https://bugzilla.redhat.com/show_bug.cgi?id=835476
> 
> 
> 
> 
> Dan

This whole block would get added to the commit message. You should not
put anything into it which you don't want there. You want to describe
the change briefly, but that's it.


> 
> ---
>  tools/virsh-domain.c | 68
> ++--
>  1 file changed, 55 insertions(+), 13 deletions(-)

This patch is corrupted. Usually it's the best to use git-send-email to
post patches. Alternatively you can attach the file itself. Pasting the
patch into your mail client will corrupt it most probably.

Please re-send a non-corrupted version with your proper name as the
author.

Also if you didn't make so, make sure you run make check and make
syntax-check before posting patches.

> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index db8accfe4..9ac855b19 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -60,6 +60,7 @@
>  #include "virsh-nodedev.h"
>  #include "viruri.h"
> 
> +

Spurious whitespace addition.

>  /* Gnulib doesn't guarantee SA_SIGINFO support.  */
>  #ifndef SA_SIGINFO
>  # define SA_SIGINFO 0
> @@ -9811,9 +9812,13 @@ static const vshCmdOptDef opts_domxmltonative[] = {
>   .flags = VSH_OFLAG_REQ,
>   .help = N_("target config data type format")
>  },
> +{.name = "domain",
> + .type = VSH_OT_DATA,
> + .flags = VSH_OFLAG_REQ_OPT,
> + .help = N_("domain name, id or uuid")
> +},
>  {.name = "xml",
>   .type = VSH_OT_DATA,
> - .flags = VSH_OFLAG_REQ,
>   .help = N_("xml data file to export from")
>  },
>  {.name = NULL}
> @@ -9822,31 +9827,68 @@ static const vshCmdOptDef opts_domxmltonative[] = {
>  static bool
>  cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
>  {
> -bool ret = true;
>  const char *format = NULL;
>  const char *xmlFile = NULL;
> -char *configData;
> -char *xmlData;
> +const char *domain = NULL;
> +char *configData = NULL;
> +char *xmlData = NULL;
>  unsigned int flags = 0;
> +unsigned int domflags = 0;
>  virshControlPtr priv = ctl->privData;
> +virDomainPtr dom = NULL;
> 
> -if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> +if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
>  return false;
> +
> +if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> + return false;
> 
> -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> -return false;
> +if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> + return false;
> +
> +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xmlFile);
> +
> +if (domain) {
> + domflags = VIRSH_BYID | VIRSH_BYUUID | VIRSH_BYNAME;
> +dom = virshLookupDomainBy(ctl, domain, domflags);

You can use virshCommandOptDomain instead of this. And the string
lookup.

> +}
> +
> +if (!dom && !xmlFile) {


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

Re: [libvirt] [PATCH 0/2] qemu: migration: bugfixes for cancelling drive mirror

[Nikolay Shirokovskiy]

ping

On 07.04.2017 14:06, Nikolay Shirokovskiy wrote:
> Nikolay Shirokovskiy (2):
>   qemu: take current async job into account in qemuBlockNodeNamesDetect
>   qemu: migration: fix race on cancelling drive mirror
> 
>  src/qemu/qemu_block.c |  6 --
>  src/qemu/qemu_block.h |  4 +++-
>  src/qemu/qemu_blockjob.c  |  9 ++---
>  src/qemu/qemu_blockjob.h  |  4 
>  src/qemu/qemu_driver.c| 11 ++-
>  src/qemu/qemu_migration.c | 43 +++
>  src/qemu/qemu_process.c   |  2 +-
>  7 files changed, 55 insertions(+), 24 deletions(-)
> 

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