Re: [libvirt] [PATCH 2/6] conf: Add invalid secrettype checks

[Peter Krempa]

On Thu, Sep 14, 2017 at 14:03:06 -0400, John Ferlan wrote:
> Add a couple of tests to "validate" checks in domain_conf that either

The quotes seem slightly inapropriate here. If this only "validates" the
checks, then why to add it? Just validate them.

> a missing secrettype (CONFIG_UNSUPPORTED) or an mismatched secrettype
> of ceph for an iSCSI disk (INTERNAL_ERROR) will cause a parsing error.
> 
> Signed-off-by: John Ferlan 
> ---
>  ...drive-network-iscsi-auth-secrettype-invalid.xml | 33 
> ++
>  ...k-drive-network-iscsi-auth-wrong-secrettype.xml | 33 
> ++
>  tests/qemuxml2argvtest.c   |  2 ++
>  3 files changed, 68 insertions(+)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-secrettype-invalid.xml
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-wrong-secrettype.xml

ACK


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

Re: [libvirt] [PATCH 1/6] docs: Remove unnecessary example for iscsi disk type='volume'

[Peter Krempa]

On Thu, Sep 14, 2017 at 14:03:05 -0400, John Ferlan wrote:
> Alter the example to remove the  from:
> 
>   
> 
> 
> 
>   
> 
> 
>   
> 
> and
> 
>   
> 
> 
> 
>   
> 
> 
>   
> 
> The reality is, it's not even used. For a  the authdef
> from the storage source pool will supercede whatever is in the 
> definition during virStorageTranslateDiskSourcePool processing. In fact,
> if the pool doesn't have/need authentication, then the authdef would
> be removed anyway as the storage pool would be handling things.
> 
> The "proof" for this is in the adjustment to the test to add an
>  for a disk. The resulting .args file won't add what normally
> would be added "myname:encodedpassword@" prior to the hostname in
> the IQN (e.g. iscsi://myname:encodedpassw...@iscsi.example.org:3260/...
> 
> Signed-off-by: John Ferlan 
> ---
>  docs/formatdomain.html.in   |  6 --
>  .../qemuxml2argv-disk-source-pool-mode.args |  3 +++
>  .../qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml | 13 
> +
>  .../qemuxml2xmlout-disk-source-pool-mode.xml| 13 
> +
>  4 files changed, 29 insertions(+), 6 deletions(-)

ACK


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

Re: [libvirt] [PATCH 6/6] conf: Use virXMLFormatElement to format disk source network

[Peter Krempa]

On Thu, Sep 14, 2017 at 14:03:10 -0400, John Ferlan wrote:
> Commit id 'e02ff020cac' neglected to use the attrBuf and childBuf
> in the virDomainDiskSourceFormatNetwork call.
> 
> So make the necessary alterations to allow usage.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/domain_conf.c | 34 ++
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 09c5bc1ae..a8771a3a4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -21674,13 +21674,14 @@ virDomainSourceDefFormatSeclabel(virBufferPtr buf,
>  
>  
>  static int
> -virDomainDiskSourceFormatNetwork(virBufferPtr buf,
> +virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
> + virBufferPtr childBuf,
>   virStorageSourcePtr src)
>  {
>  size_t n;
>  char *path = NULL;
>  
> -virBufferAsprintf(buf, " +virBufferAsprintf(attrBuf, " protocol='%s'",
>virStorageNetProtocolTypeToString(src->protocol));
>  
>  if (src->volume) {
> @@ -21688,36 +21689,29 @@ virDomainDiskSourceFormatNetwork(virBufferPtr buf,
>  return -1;
>  }
>  
> -virBufferEscapeString(buf, " name='%s'", path ? path : src->path);
> +virBufferEscapeString(attrBuf, " name='%s'", path ? path : src->path);
>  
>  VIR_FREE(path);
>  
> -if (src->nhosts == 0 && !src->snapshot && !src->configFile) {
> -virBufferAddLit(buf, "/>\n");
> -} else {
> -virBufferAddLit(buf, ">\n");
> -virBufferAdjustIndent(buf, 2);
> +if (src->nhosts > 0 || src->snapshot || src->configFile) {

This condition isn't necessary as well after these adjustments, drop it
and un-indent the block below.

>  
>  for (n = 0; n < src->nhosts; n++) {
> -virBufferAddLit(buf, " -virBufferEscapeString(buf, " name='%s'", src->hosts[n].name);
> +virBufferAddLit(childBuf, " +virBufferEscapeString(childBuf, " name='%s'", 
> src->hosts[n].name);
>  
>  if (src->hosts[n].port)
> -virBufferAsprintf(buf, " port='%u'", src->hosts[n].port);
> +virBufferAsprintf(childBuf, " port='%u'", 
> src->hosts[n].port);
>  
>  if (src->hosts[n].transport)
> -virBufferAsprintf(buf, " transport='%s'",
> +virBufferAsprintf(childBuf, " transport='%s'",
>
> virStorageNetHostTransportTypeToString(src->hosts[n].transport));
>  
> -virBufferEscapeString(buf, " socket='%s'", src->hosts[n].socket);
> -virBufferAddLit(buf, "/>\n");
> +virBufferEscapeString(childBuf, " socket='%s'", 
> src->hosts[n].socket);
> +virBufferAddLit(childBuf, "/>\n");
>  }
>  
> -virBufferEscapeString(buf, "\n", src->snapshot);
> -virBufferEscapeString(buf, "\n", src->configFile);
> -
> -virBufferAdjustIndent(buf, -2);
> -virBufferAddLit(buf, "\n");
> +virBufferEscapeString(childBuf, "\n", 
> src->snapshot);
> +virBufferEscapeString(childBuf, "\n", 
> src->configFile);
>  }
>  
>  return 0;

ACK with the tweak above.


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

Re: [libvirt] [PATCH 4/6] conf: Add invalid domain disk encryption test

[Peter Krempa]

On Thu, Sep 14, 2017 at 14:03:08 -0400, John Ferlan wrote:
> Add a test to prove checking for invalid luks disk formatting check

According to the error message the  element is not necessary
when defining a disk for a domain:

419) QEMU XML-2-ARGV luks-disk-invalid ... Got 
expected error: 
2017-09-15 03:59:41.787+: 237800: error : virDomainDiskDefParseXML:9104 : 
unsupported configuration: supplying the  for a domain is unnecessary

Please note in the commit message that the  element is to look
for.

> Signed-off-by: John Ferlan 
> ---
>  .../qemuxml2argv-luks-disk-invalid.xml | 37 
> ++
>  tests/qemuxml2argvtest.c   |  1 +
>  2 files changed, 38 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-invalid.xml

ACK


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

Re: [libvirt] [PATCH 5/6] conf: Move encryption validation

[Peter Krempa]

On Thu, Sep 14, 2017 at 14:03:09 -0400, John Ferlan wrote:
> Rather than checking during XML processing, move the check for
> valid  into virDomainDiskDefParseValidate.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/domain_conf.c | 29 +
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 07bda1a36..09c5bc1ae 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8605,7 +8605,23 @@ virDomainDiskDefParseValidate(const virDomainDiskDef 
> *def)
>  }
>  }
>  
> -return virDomainDiskSourceDefParseAuthValidate(def->src);
> +if (virDomainDiskSourceDefParseAuthValidate(def->src) < 0)

This is the exact reason why I did not like this style in the patch that
added it.

> +return -1;
> +
> +if (def->src->encryption) {
> +virStorageEncryptionPtr encryption = def->src->encryption;
> +
> +if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
> +encryption->encinfo.cipher_name) {
> +
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("supplying the  for a domain is "
> + "unnecessary"));

Perhaps we can improve the message now. How about:

"supplying  for domain disk definition is unnecessary" ?

> +return -1;
> +}

ACK


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

Re: [libvirt] [PATCH 3/6] conf: Move authdef validation

[Peter Krempa]

On Thu, Sep 14, 2017 at 14:03:07 -0400, John Ferlan wrote:
> Rather than checking during XML processing, move the checks for correct
> and valid auth into virDomainDiskDefParseValidate. This will introduce
> virDomainDiskSourceDefParseAuthValidate to validate that the authdef
> stored for the virStorageSource is valid. This can then be expanded
> to service backingStore sources as well.
> 
> Alter the message text slightly as well to distinguish between an
> unknown name and an incorrectly used name.  Since type is not a
> mandatory field, add the NULLSTR() around the output of the unknown
> error. NB, a config using unknown formatting would fail virschematest
> since it only accepts 'iscsi' and 'ceph' as "valid" types.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/domain_conf.c | 67 
> +-
>  1 file changed, 34 insertions(+), 33 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a43b25c31..07bda1a36 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8500,6 +8500,39 @@ virDomainDiskDefGeometryParse(virDomainDiskDefPtr def,
>  
>  
>  static int
> +virDomainDiskSourceDefParseAuthValidate(const virStorageSource *src)
> +{
> +virStorageAuthDefPtr authdef = src->auth;
> +int actUsage;
> +
> +/* Disk volume types won't have the secrettype filled in until
> + * after virStorageTranslateDiskSourcePool is run
> + */
> +if (src->type == VIR_STORAGE_TYPE_VOLUME || !authdef)
> +return 0;

Should this also include || src->type != VIR_STORAGE_TYPE_NETWORK?

> +
> +if ((actUsage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("unknown secret type '%s'"),
> +   NULLSTR(authdef->secrettype));
> +return -1;
> +}
> +
> +if ((src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
> + actUsage != VIR_SECRET_USAGE_TYPE_ISCSI) ||
> +(src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
> + actUsage != VIR_SECRET_USAGE_TYPE_CEPH)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("invalid secret type '%s'"),
> +   virSecretUsageTypeToString(actUsage));
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +
> +static int
>  virDomainDiskDefParseValidate(const virDomainDiskDef *def)
>  {
>  if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) {
> @@ -8572,7 +8605,7 @@ virDomainDiskDefParseValidate(const virDomainDiskDef 
> *def)
>  }
>  }
>  
> -return 0;
> +return virDomainDiskSourceDefParseAuthValidate(def->src);

As common in similar functions, add a if block and leave the "return 0"
intact. Saving two lines is not worth breaking the structure and making
the code less extensible.

>  }
>  
>  

ACK with the above fixed.


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

Re: [libvirt] [PATCH] spec: Own %{_libdir}/libvirt{, /connection-driver} dirs, mark licenses as %license

[Cole Robinson]

On 08/28/2017 03:15 AM, Peter Krempa wrote:
> On Sun, Aug 27, 2017 at 12:35:07 -0400, Cole Robinson wrote:
>> From: Ville Skyttä 
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1483293
>> Signed-off-by: Cole Robinson 
>> ---
> 
> Looks like two separate problems. Also the commit message is rather
> sparse.
> 
>>  libvirt.spec.in | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index 2b58bf3a9..2d21e60bc 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -1794,6 +1794,8 @@ exit 0
>>  %dir %attr(0711, root, root) %{_localstatedir}/cache/libvirt/
>>  
>>  
>> +%dir %attr(0755, root, root) %{_libdir}/libvirt/
>> +%dir %attr(0755, root, root) %{_libdir}/libvirt/connection-driver/
>>  %dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
>>  %attr(0755, root, root) %{_libdir}/libvirt/lock-driver/lockd.so
>>  
>> @@ -2028,7 +2030,7 @@ exit 0
>>  %attr(0755, root, root) %{_libexecdir}/libvirt-guests.sh
>>  
>>  %files libs -f %{name}.lang
>> -%doc COPYING COPYING.LESSER
>> +%license COPYING COPYING.LESSERa
> 
> According to https://bugzilla.redhat.com/show_bug.cgi?id=1396828 ,
> %license will not work on rhel/centos 6.
> 
> You should add the hack described in the BZ. Or not change it at all,
> since it's not clear from this commit or the bugzilla why it's needed.
> 

Thanks for the review and sorry for not checking %license closely. I've sent
split apart patches with some more info, and %license back compat

Thanks,
Cole

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

[libvirt] [PATCH] spec: Use %license when available

[Cole Robinson]

This is required by the fedora packaging guidelines:

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines

This macro isn't available on stock RHEL6 so provide a backcompat
definition

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

Reported-by: Ville Skyttä 
Signed-off-by: Cole Robinson 
---
 libvirt.spec.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 583fb0583..5f232b1ba 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -15,6 +15,8 @@
 # touch configure.ac or Makefile.am.
 %{!?enable_autotools:%global enable_autotools 0}
 
+# RHEL6 doesn't have 'license' macro
+%{!?_licensedir:%global license %%doc}
 
 # The hypervisor drivers that run in libvirtd
 %define with_xen   0%{!?_without_xen:1}
@@ -2030,7 +2032,7 @@ exit 0
 %attr(0755, root, root) %{_libexecdir}/libvirt-guests.sh
 
 %files libs -f %{name}.lang
-%doc COPYING COPYING.LESSER
+%license COPYING COPYING.LESSER
 %config(noreplace) %{_sysconfdir}/libvirt/libvirt.conf
 %config(noreplace) %{_sysconfdir}/libvirt/libvirt-admin.conf
 %{_libdir}/libvirt.so.*
-- 
2.13.5

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

[libvirt] [PATCH] spec: Own %{_libdir}/libvirt{, /connection-driver} dirs

[Cole Robinson]

From: Ville Skyttä 

Owning all created directories is a requirement of the Fedora
packaging guidelines

https://bugzilla.redhat.com/show_bug.cgi?id=1483293
Signed-off-by: Cole Robinson 
---
 libvirt.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2b58bf3a9..583fb0583 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1794,6 +1794,8 @@ exit 0
 %dir %attr(0711, root, root) %{_localstatedir}/cache/libvirt/
 
 
+%dir %attr(0755, root, root) %{_libdir}/libvirt/
+%dir %attr(0755, root, root) %{_libdir}/libvirt/connection-driver/
 %dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
 %attr(0755, root, root) %{_libdir}/libvirt/lock-driver/lockd.so
 
-- 
2.13.5

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

[libvirt] [PATCH 5/6] conf: Move encryption validation

[John Ferlan]

Rather than checking during XML processing, move the check for
valid  into virDomainDiskDefParseValidate.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 07bda1a36..09c5bc1ae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8605,7 +8605,23 @@ virDomainDiskDefParseValidate(const virDomainDiskDef 
*def)
 }
 }
 
-return virDomainDiskSourceDefParseAuthValidate(def->src);
+if (virDomainDiskSourceDefParseAuthValidate(def->src) < 0)
+return -1;
+
+if (def->src->encryption) {
+virStorageEncryptionPtr encryption = def->src->encryption;
+
+if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+encryption->encinfo.cipher_name) {
+
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("supplying the  for a domain is "
+ "unnecessary"));
+return -1;
+}
+}
+
+return 0;
 }
 
 
@@ -9095,17 +9111,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 def->startupPolicy = val;
 }
 
-if (encryption) {
-if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
-encryption->encinfo.cipher_name) {
-
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("supplying the  for a domain is "
- "unnecessary"));
-goto error;
-}
-}
-
 def->dst = target;
 target = NULL;
 def->src->auth = authdef;
-- 
2.13.5

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

[libvirt] [PATCH 2/6] conf: Add invalid secrettype checks

[John Ferlan]

Add a couple of tests to "validate" checks in domain_conf that either
a missing secrettype (CONFIG_UNSUPPORTED) or an mismatched secrettype
of ceph for an iSCSI disk (INTERNAL_ERROR) will cause a parsing error.

Signed-off-by: John Ferlan 
---
 ...drive-network-iscsi-auth-secrettype-invalid.xml | 33 ++
 ...k-drive-network-iscsi-auth-wrong-secrettype.xml | 33 ++
 tests/qemuxml2argvtest.c   |  2 ++
 3 files changed, 68 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-secrettype-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-wrong-secrettype.xml

diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-secrettype-invalid.xml
 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-secrettype-invalid.xml
new file mode 100644
index 0..7e6b623c3
--- /dev/null
+++ 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-secrettype-invalid.xml
@@ -0,0 +1,33 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+
+  
+  
+
+  
+  
+
+
+
+
+
+
+  
+
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-wrong-secrettype.xml
 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-wrong-secrettype.xml
new file mode 100644
index 0..4854abd6c
--- /dev/null
+++ 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-wrong-secrettype.xml
@@ -0,0 +1,33 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+
+  
+  
+
+  
+  
+
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 2c040e4c0..fd05155ef 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -917,6 +917,8 @@ mymain(void)
 DO_TEST("disk-drive-network-nbd-unix", NONE);
 DO_TEST("disk-drive-network-iscsi", NONE);
 DO_TEST("disk-drive-network-iscsi-auth", NONE);
+DO_TEST_PARSE_ERROR("disk-drive-network-iscsi-auth-secrettype-invalid", 
NONE);
+DO_TEST_PARSE_ERROR("disk-drive-network-iscsi-auth-wrong-secrettype", 
NONE);
 DO_TEST("disk-drive-network-iscsi-lun",
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI,
 QEMU_CAPS_SCSI_BLOCK);
-- 
2.13.5

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

[libvirt] [PATCH 0/6] Some disk auth/encryption cleanups

[John Ferlan]

Perform some cleanups with the auth and encryption processing.

This is a precursor to some other changes that will move/create
disk  level  and  elements since they
are already in the _virStorageSource.

I'm still working on the latter, but before there's too many patches.

John Ferlan (6):
  docs: Remove unnecessary  example for iscsi disk type='volume'
  conf: Add invalid secrettype checks
  conf: Move  authdef validation
  conf: Add invalid domain disk encryption test
  conf: Move  encryption validation
  conf: Use virXMLFormatElement to format disk source network

 docs/formatdomain.html.in  |   6 -
 src/conf/domain_conf.c | 126 ++---
 ...drive-network-iscsi-auth-secrettype-invalid.xml |  33 ++
 ...k-drive-network-iscsi-auth-wrong-secrettype.xml |  33 ++
 .../qemuxml2argv-disk-source-pool-mode.args|   3 +
 .../qemuxml2argv-disk-source-pool-mode.xml |  13 +++
 .../qemuxml2argv-luks-disk-invalid.xml |  37 ++
 tests/qemuxml2argvtest.c   |   3 +
 .../qemuxml2xmlout-disk-source-pool-mode.xml   |  13 +++
 9 files changed, 198 insertions(+), 69 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-secrettype-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-wrong-secrettype.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-invalid.xml

-- 
2.13.5

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

[libvirt] [PATCH 6/6] conf: Use virXMLFormatElement to format disk source network

[John Ferlan]

Commit id 'e02ff020cac' neglected to use the attrBuf and childBuf
in the virDomainDiskSourceFormatNetwork call.

So make the necessary alterations to allow usage.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 34 ++
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 09c5bc1ae..a8771a3a4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21674,13 +21674,14 @@ virDomainSourceDefFormatSeclabel(virBufferPtr buf,
 
 
 static int
-virDomainDiskSourceFormatNetwork(virBufferPtr buf,
+virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
+ virBufferPtr childBuf,
  virStorageSourcePtr src)
 {
 size_t n;
 char *path = NULL;
 
-virBufferAsprintf(buf, "protocol));
 
 if (src->volume) {
@@ -21688,36 +21689,29 @@ virDomainDiskSourceFormatNetwork(virBufferPtr buf,
 return -1;
 }
 
-virBufferEscapeString(buf, " name='%s'", path ? path : src->path);
+virBufferEscapeString(attrBuf, " name='%s'", path ? path : src->path);
 
 VIR_FREE(path);
 
-if (src->nhosts == 0 && !src->snapshot && !src->configFile) {
-virBufferAddLit(buf, "/>\n");
-} else {
-virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
+if (src->nhosts > 0 || src->snapshot || src->configFile) {
 
 for (n = 0; n < src->nhosts; n++) {
-virBufferAddLit(buf, "hosts[n].name);
+virBufferAddLit(childBuf, "hosts[n].name);
 
 if (src->hosts[n].port)
-virBufferAsprintf(buf, " port='%u'", src->hosts[n].port);
+virBufferAsprintf(childBuf, " port='%u'", src->hosts[n].port);
 
 if (src->hosts[n].transport)
-virBufferAsprintf(buf, " transport='%s'",
+virBufferAsprintf(childBuf, " transport='%s'",
   
virStorageNetHostTransportTypeToString(src->hosts[n].transport));
 
-virBufferEscapeString(buf, " socket='%s'", src->hosts[n].socket);
-virBufferAddLit(buf, "/>\n");
+virBufferEscapeString(childBuf, " socket='%s'", 
src->hosts[n].socket);
+virBufferAddLit(childBuf, "/>\n");
 }
 
-virBufferEscapeString(buf, "\n", src->snapshot);
-virBufferEscapeString(buf, "\n", src->configFile);
-
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+virBufferEscapeString(childBuf, "\n", 
src->snapshot);
+virBufferEscapeString(childBuf, "\n", 
src->configFile);
 }
 
 return 0;
@@ -21766,7 +21760,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
 break;
 
 case VIR_STORAGE_TYPE_NETWORK:
-if (virDomainDiskSourceFormatNetwork(buf, src) < 0)
+if (virDomainDiskSourceFormatNetwork(, , src) < 0)
 goto error;
 break;
 
-- 
2.13.5

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

[libvirt] [PATCH 3/6] conf: Move authdef validation

[John Ferlan]

Rather than checking during XML processing, move the checks for correct
and valid auth into virDomainDiskDefParseValidate. This will introduce
virDomainDiskSourceDefParseAuthValidate to validate that the authdef
stored for the virStorageSource is valid. This can then be expanded
to service backingStore sources as well.

Alter the message text slightly as well to distinguish between an
unknown name and an incorrectly used name.  Since type is not a
mandatory field, add the NULLSTR() around the output of the unknown
error. NB, a config using unknown formatting would fail virschematest
since it only accepts 'iscsi' and 'ceph' as "valid" types.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 67 +-
 1 file changed, 34 insertions(+), 33 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a43b25c31..07bda1a36 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8500,6 +8500,39 @@ virDomainDiskDefGeometryParse(virDomainDiskDefPtr def,
 
 
 static int
+virDomainDiskSourceDefParseAuthValidate(const virStorageSource *src)
+{
+virStorageAuthDefPtr authdef = src->auth;
+int actUsage;
+
+/* Disk volume types won't have the secrettype filled in until
+ * after virStorageTranslateDiskSourcePool is run
+ */
+if (src->type == VIR_STORAGE_TYPE_VOLUME || !authdef)
+return 0;
+
+if ((actUsage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown secret type '%s'"),
+   NULLSTR(authdef->secrettype));
+return -1;
+}
+
+if ((src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
+ actUsage != VIR_SECRET_USAGE_TYPE_ISCSI) ||
+(src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
+ actUsage != VIR_SECRET_USAGE_TYPE_CEPH)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid secret type '%s'"),
+   virSecretUsageTypeToString(actUsage));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
 virDomainDiskDefParseValidate(const virDomainDiskDef *def)
 {
 if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) {
@@ -8572,7 +8605,7 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def)
 }
 }
 
-return 0;
+return virDomainDiskSourceDefParseAuthValidate(def->src);
 }
 
 
@@ -8731,8 +8764,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *vendor = NULL;
 char *product = NULL;
 char *domain_name = NULL;
-int expected_secret_usage = -1;
-int auth_secret_usage = -1;
 
 if (!(def = virDomainDiskDefNew(xmlopt)))
 return NULL;
@@ -8776,13 +8807,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 
 source = true;
 
-if (def->src->type == VIR_STORAGE_TYPE_NETWORK) {
-if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI)
-expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI;
-else if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)
-expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH;
-}
-
 startupPolicy = virXMLPropString(cur, "startupPolicy");
 
 } else if (!target &&
@@ -8840,17 +8864,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
virXMLNodeNameEqual(cur, "auth")) {
 if (!(authdef = virStorageAuthDefParse(node->doc, cur)))
 goto error;
-/* Disk volume types won't have the secrettype filled in until
- * after virStorageTranslateDiskSourcePool is run
- */
-if (def->src->type != VIR_STORAGE_TYPE_VOLUME &&
-(auth_secret_usage =
- virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("invalid secret type %s"),
-   authdef->secrettype);
-goto error;
-}
 } else if (virXMLNodeNameEqual(cur, "iotune")) {
 if (virDomainDiskDefIotuneParse(def, ctxt) < 0)
 goto error;
@@ -8914,18 +8927,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 }
 
-/* Disk volume types will have authentication information handled in
- * virStorageTranslateDiskSourcePool
- */
-if (def->src->type != VIR_STORAGE_TYPE_VOLUME &&
-auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage) 
{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("invalid secret type '%s'"),
-   virSecretUsageTypeToString(auth_secret_usage));
-goto error;
-}
-
-
 /* Only CDROM and Floppy devices are allowed missing source path
  * to indicate no media present. LUN is for raw access CD-ROMs

[libvirt] [PATCH 4/6] conf: Add invalid domain disk encryption test

[John Ferlan]

Add a test to prove checking for invalid luks disk formatting check

Signed-off-by: John Ferlan 
---
 .../qemuxml2argv-luks-disk-invalid.xml | 37 ++
 tests/qemuxml2argvtest.c   |  1 +
 2 files changed, 38 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-invalid.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-invalid.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-invalid.xml
new file mode 100644
index 0..bea769584
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-invalid.xml
@@ -0,0 +1,37 @@
+
+  encryptdisk
+  496898a6-e6ff-f7c8-5dc2-3cf410945ee9
+  1048576
+  524288
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+  
+  
+
+
+  
+  
+
+
+  
+
+
+
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index fd05155ef..c8c479cbd 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1647,6 +1647,7 @@ mymain(void)
 # else
 DO_TEST_FAILURE("luks-disks", QEMU_CAPS_OBJECT_SECRET);
 # endif
+DO_TEST_PARSE_ERROR("luks-disk-invalid", NONE);
 
 DO_TEST("memtune", NONE);
 DO_TEST("memtune-unlimited", NONE);
-- 
2.13.5

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

[libvirt] [PATCH 1/6] docs: Remove unnecessary example for iscsi disk type='volume'

[John Ferlan]

Alter the example to remove the  from:

  



  


  

and

  



  


  

The reality is, it's not even used. For a  the authdef
from the storage source pool will supercede whatever is in the 
definition during virStorageTranslateDiskSourcePool processing. In fact,
if the pool doesn't have/need authentication, then the authdef would
be removed anyway as the storage pool would be handling things.

The "proof" for this is in the adjustment to the test to add an
 for a disk. The resulting .args file won't add what normally
would be added "myname:encodedpassword@" prior to the hostname in
the IQN (e.g. iscsi://myname:encodedpassw...@iscsi.example.org:3260/...

Signed-off-by: John Ferlan 
---
 docs/formatdomain.html.in   |  6 --
 .../qemuxml2argv-disk-source-pool-mode.args |  3 +++
 .../qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml | 13 +
 .../qemuxml2xmlout-disk-source-pool-mode.xml| 13 +
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8ca7637a4..3b78bbeb8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2385,17 +2385,11 @@
   disk type='volume' device='disk'
 driver name='qemu' type='raw'/
 source pool='iscsi-pool' volume='unit:0:0:1' mode='host'/
-auth username='myuser'
-  secret type='iscsi' usage='libvirtiscsi'/
-/auth
 target dev='vdb' bus='virtio'/
   /disk
   disk type='volume' device='disk'
 driver name='qemu' type='raw'/
 source pool='iscsi-pool' volume='unit:0:0:2' mode='direct'/
-auth username='myuser'
-  secret type='iscsi' usage='libvirtiscsi'/
-/auth
 target dev='vdc' bus='virtio'/
   /disk
   disk type='file' device='disk'
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
index 7cda627f2..5b4e65e10 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
@@ -28,4 +28,7 @@ id=drive-ide0-0-2,readonly=on \
 -device ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 \
 -drive file=/tmp/idedisk.img,format=raw,if=none,id=drive-ide0-0-3 \
 -device ide-drive,bus=ide.0,unit=3,drive=drive-ide0-0-3,id=ide0-0-3 \
+-drive file=iscsi://iscsi.example.com:3260/demo-target/3,if=none,media=cdrom,\
+id=drive-ide0-0-4,readonly=on \
+-device ide-drive,bus=ide.0,unit=4,drive=drive-ide0-0-4,id=ide0-0-4 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
index eaf411c8b..3f5a2d524 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
@@ -39,6 +39,19 @@
   
   
 
+
+  
+
+  
+  
+
+  system_u:system_r:public_content_t:s0
+
+  
+  
+  
+  
+
 
 
 
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool-mode.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool-mode.xml
index 1ca56fbb9..a14ed7b97 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool-mode.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool-mode.xml
@@ -39,6 +39,19 @@
   
   
 
+
+  
+
+  
+  
+
+  system_u:system_r:public_content_t:s0
+
+  
+  
+  
+  
+
 
   
 
-- 
2.13.5

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

Re: [libvirt] QEMU -M nvdimm=on and hotplug

[Stefan Hajnoczi]

On Wed, Sep 13, 2017 at 05:28:56PM +0200, Michal Privoznik wrote:
> On 09/13/2017 03:54 PM, Stefan Hajnoczi wrote:
> > 2. Only allow NVDIMM hotplug if the domain was started with -M
> >nvdimm=on.
> > 
> > I think QEMU will not add -M nvdimm=on to the "pc" machine type by
> > default since it adds the NVDIMM DSM hardware interface that increases
> > the security attack surface.
> 
> So this is the downside. Well, I think all that we are left with is
> option 2 then. Or, we can expose nvdimm=on the domain XML (and enable it
> by default for any domain that already has a nvdimm device configured).

There probably needs to be an explicit nvdimm=on XML element so that
domains can be created without NVDIMMs at boot but the ability to
hotplug NVDIMMs later.

Stefan

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

Re: [libvirt] [PATCH 8/8] travis: Shuffle sections around

[Andrea Bolognani]

On Thu, 2017-09-14 at 17:31 +0200, Martin Kletzander wrote:
> On Thu, Sep 14, 2017 at 03:09:08PM +0200, Andrea Bolognani wrote:
> > Order them more logically and make sure that stuff that doesn't
> > need to be modified frequently if at all, such as the notification
> > settings, are out of the way.
> > 
> > Perform other minor tweaks as well.
> 
> Two whitespaces are less than "minor" I'd say :D

Based on your feedback, I changed it to "very minor" before pushing.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 8/8] travis: Shuffle sections around

[Martin Kletzander]

On Thu, Sep 14, 2017 at 03:09:08PM +0200, Andrea Bolognani wrote:

Order them more logically and make sure that stuff that doesn't
need to be modified frequently if at all, such as the notification
settings, are out of the way.

Perform other minor tweaks as well.

Signed-off-by: Andrea Bolognani 
---
.travis.yml | 62 +++--
1 file changed, 32 insertions(+), 30 deletions(-)



Two whitespaces are less than "minor" I'd say :D

Reviewed-by: Martin Kletzander 


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

Re: [libvirt] [PATCH 4/8] travis: Split building command

[Martin Kletzander]

On Thu, Sep 14, 2017 at 05:10:24PM +0200, Andrea Bolognani wrote:

On Thu, 2017-09-14 at 16:49 +0200, Martin Kletzander wrote:

> So I'm on the fence about this patch - I'd have a slight preference for
> existing behaviour of failing fast to keep errors at the end of the log

I, personally, would go with:

script:
 - make -j3 check
 - make -j3 syntax-check

The former executes the whole build and fails on first compilation or
test error.  The second one has a format that's easily skippable
(visually) and that's not that much to scroll through.


We can't run either 'make check' or 'make syntax-check' on macOS
though, so your proposal would effectively kill all testing on that
platform :)



Well, but that's changed after this patch, so technically it wouldn't ;)
Anyway, the point was keeping syntax-check last as that's easier to
visually skip.

ACK either way


--
Andrea Bolognani / Red Hat / Virtualization

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


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

Re: [libvirt] [PATCHv2] nodedev: add switchdev to NIC capabilities

[Laine Stump]

(Almost all of my comments result in "ok, no action needed". There are
just three items I would like to see changed (2 trivial, 1 also small
but Edan or John may think it prudent to re-test with the change before
pushing) - I marked those comments with [**].

On 08/21/2017 05:19 AM, Edan David wrote:
> Adding functionality to libvirt that will allow querying the interface
> for the availability of switchdev Offloading NIC capabilities.
> The switchdev mode was introduced in kernel 4.8, the iproute2-devlink
> command to retrieve the swtichdev NIC feature,
> Command example:  devlink dev eswitch show pci/:03:00.0
> This feature is needed for Openstack so we can do a scheduling decision
> if the NIC is in Hardware Offload (switchdev) or regular SR-IOV (legacy) mode.
> And select the appropriate hypervisors with the requested capability see [1].
>
> [1] - 
> https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/enable-sriov-nic-features.html
> ---
>  configure.ac  |  13 ++
>  docs/formatnode.html.in   |   1 +
>  src/util/virnetdev.c  | 187 
> +-
>  src/util/virnetdev.h  |   1 +
>  tests/nodedevschemadata/net_00_13_02_b9_f9_d3.xml |   1 +
>  tests/nodedevschemadata/net_00_15_58_2f_e9_55.xml |   1 +
>  6 files changed, 203 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index b12b7fa..c089798 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -627,6 +627,19 @@ if test "$with_linux" = "yes"; then
>  AC_CHECK_HEADERS([linux/btrfs.h])
>  fi
>  
> +dnl
> +dnl check for kernel headers required by devlink
> +dnl
> +if test "$with_linux" = "yes"; then
> +AC_CHECK_HEADERS([linux/devlink.h])
> +AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, 
> DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, 
> DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, 
> DEVLINK_ESWITCH_MODE_SWITCHDEV],

That's very . thorough, and potentially misleading if someone ever
wanted to use devlink to check for something other than switchdev (e.g.
[mythical feature X]) on some system that didn't have the proper stuff
defined for switchdev, but did have it for [other mythical feature X].
But that's all just hypothetical, so this is fine with me.


> +   [AC_DEFINE([HAVE_DECL_DEVLINK],
> +  [1],
> +  [whether devlink declarations is available])],

[**]
s/is/are/

> +   [],
> +   [[#include ]])
> +fi
> +
>  dnl Allow perl/python overrides
>  AC_PATH_PROGS([PYTHON], [python2 python])
>  if test -z "$PYTHON"; then
> diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
> index 4d935b5..29244a8 100644
> --- a/docs/formatnode.html.in
> +++ b/docs/formatnode.html.in
> @@ -227,6 +227,7 @@
>  rxhashreceive-hashing
>  
> rdmaremote-direct-memory-access
>  
> txudptnltx-udp-tunnel-segmentation
> +
> switchdevkernel-forward-plane-offload

pretty odd abbreviation. But it is what it is :-)

>  
>
>capability
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 51a6e42..fc7c961 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -59,6 +59,10 @@
>  # include 
>  #endif
>  
> +#if HAVE_DECL_DEVLINK
> +# include 
> +#endif
> +
>  #ifndef IFNAMSIZ
>  # define IFNAMSIZ 16
>  #endif
> @@ -2481,7 +2485,8 @@ VIR_ENUM_IMPL(virNetDevFeature,
>"ntuple",
>"rxhash",
>"rdma",
> -  "txudptnl")
> +  "txudptnl",
> +  "switchdev")
>  
>  #ifdef __linux__
>  int
> @@ -2936,6 +2941,7 @@ int virNetDevGetRxFilter(const char *ifname,
>  return ret;
>  }
>  
> +

[**]
Added a spurious extra line.

>  #if defined(SIOCETHTOOL) && defined(HAVE_STRUCT_IFREQ)
>  
>  /**
> @@ -3115,6 +3121,182 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
>  }
>  
>  
> +#if HAVE_DECL_DEVLINK
> +/**
> + * virNetDevPutExtraHeader
> + * reserve and prepare room for an extra header
> + * This function sets to zero the room that is required to put the extra
> + * header after the initial Netlink header. This function also increases
> + * the nlmsg_len field.
> + *
> + * @nlh: pointer to Netlink header
> + * @size: size of the extra header that we want to put
> + *
> + * Returns pointer to the start of the extended header
> + */
> +static void *
> +virNetDevPutExtraHeader(struct nlmsghdr *nlh,
> +size_t size)
> +{
> +char *ptr = (char *)nlh + nlh->nlmsg_len;
> +size_t len = NLMSG_ALIGN(size);
> +nlh->nlmsg_len += len;
> +memset(ptr, 0, len);

[**]
I'm fairly confident this memset() is unnecessary. The buffer the "extra
header" is being added to is allocated with nlmsg_alloc_simple(); I
looked 

Re: [libvirt] [PATCH 4/8] travis: Split building command

[Martin Kletzander]

On Thu, Sep 14, 2017 at 02:48:06PM +0100, Daniel P. Berrange wrote:

On Thu, Sep 14, 2017 at 03:09:04PM +0200, Andrea Bolognani wrote:

The build will fail if any of the commands fail, but this way we
might catch more errors in a single run.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index ba8ff49a1..c2526bc6d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -76,7 +76,9 @@ before_install:
 before_script:
   - ./autogen.sh
 script:
-  - make -j3 && make -j3 syntax-check && make -j3 check
+  - make -j3
+  - make -j3 syntax-check
+  - make -j3 check

 # Environments here are run in addition to the main environment defined above
 matrix:


The downside of this is that if syntax-check fails, but check succeeeds,
you now have to search through the middle of the logfile to find the
failure, instead of just jumping straight to the end.

So I'm on the fence about this patch - I'd have a slight preference for
existing behaviour of failing fast to keep errors at the end of the log



I, personally, would go with:

script:
- make -j3 check
- make -j3 syntax-check

The former executes the whole build and fails on first compilation or
test error.  The second one has a format that's easily skippable
(visually) and that's not that much to scroll through.

I'm leaving the choice up to you, though.


None the less

Reviewed-by: Daniel P. Berrange 

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

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


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

Re: [libvirt] [PATCH 4/8] travis: Split building command

[Andrea Bolognani]

On Thu, 2017-09-14 at 16:49 +0200, Martin Kletzander wrote:
> > So I'm on the fence about this patch - I'd have a slight preference for
> > existing behaviour of failing fast to keep errors at the end of the log
> 
> I, personally, would go with:
> 
> script:
>  - make -j3 check
>  - make -j3 syntax-check
> 
> The former executes the whole build and fails on first compilation or
> test error.  The second one has a format that's easily skippable
> (visually) and that's not that much to scroll through.

We can't run either 'make check' or 'make syntax-check' on macOS
though, so your proposal would effectively kill all testing on that
platform :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 7/8] travis: Install more build dependencies

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 04:09:37PM +0200, Andrea Bolognani wrote:
> On Thu, 2017-09-14 at 14:50 +0100, Daniel P. Berrange wrote:
> > > @@ -5,37 +5,60 @@ addons:
> > >apt:
> > >  # Please keep this list sorted alphabetically
> > >  packages:
> [...]
> > > +  - zfs-fuse
> > 
> > Did you check if travis actually lets you install all these ?  They have
> > to whitelist each individual package that's desired. So if some of these
> > aren't in the whitelist, adding them here will have no effect IIUC.
> 
> That's kind of insane.

They don't fully trust the security of the container env in which they
populate the packages I guess.

> Anyway, it looks like all the packages in the list are allowed,
> so whatever :)

Ok,

Reviewed-by: Daniel P. Berrange 

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

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

Re: [libvirt] [PATCH 7/8] travis: Install more build dependencies

[Andrea Bolognani]

On Thu, 2017-09-14 at 14:50 +0100, Daniel P. Berrange wrote:
> > @@ -5,37 +5,60 @@ addons:
> >apt:
> >  # Please keep this list sorted alphabetically
> >  packages:
[...]
> > +  - zfs-fuse
> 
> Did you check if travis actually lets you install all these ?  They have
> to whitelist each individual package that's desired. So if some of these
> aren't in the whitelist, adding them here will have no effect IIUC.

That's kind of insane.

Anyway, it looks like all the packages in the list are allowed,
so whatever :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 4/8] travis: Split building command

[Andrea Bolognani]

On Thu, 2017-09-14 at 14:48 +0100, Daniel P. Berrange wrote:
> >  script:
> > -  - make -j3 && make -j3 syntax-check && make -j3 check
> > +  - make -j3
> > +  - make -j3 syntax-check
> > +  - make -j3 check
> > 
> >  # Environments here are run in addition to the main environment defined 
> > above
> >  matrix:
> 
> The downside of this is that if syntax-check fails, but check succeeeds,
> you now have to search through the middle of the logfile to find the
> failure, instead of just jumping straight to the end.
> 
> So I'm on the fence about this patch - I'd have a slight preference for
> existing behaviour of failing fast to keep errors at the end of the log

I can drop this patch and have the end result look like

  script:
# Many unit tests still fail on macOS, and there are a bunch of issues
# with syntax-check as well, so skip them for now
- make -j3 && if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check 
&& make -j3 check; fi

Would you like that better?

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 4/8] travis: Split building command

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 04:45:30PM +0200, Andrea Bolognani wrote:
> On Thu, 2017-09-14 at 14:48 +0100, Daniel P. Berrange wrote:
> > >  script:
> > > -  - make -j3 && make -j3 syntax-check && make -j3 check
> > > +  - make -j3
> > > +  - make -j3 syntax-check
> > > +  - make -j3 check
> > > 
> > >  # Environments here are run in addition to the main environment defined 
> > > above
> > >  matrix:
> > 
> > The downside of this is that if syntax-check fails, but check succeeeds,
> > you now have to search through the middle of the logfile to find the
> > failure, instead of just jumping straight to the end.
> > 
> > So I'm on the fence about this patch - I'd have a slight preference for
> > existing behaviour of failing fast to keep errors at the end of the log
> 
> I can drop this patch and have the end result look like
> 
>   script:
> # Many unit tests still fail on macOS, and there are a bunch of issues
> # with syntax-check as well, so skip them for now
> - make -j3 && if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 
> syntax-check && make -j3 check; fi
> 
> Would you like that better?

Yep, I think so

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

[libvirt] [PATCH 3/4] cpu_s390: Implement virCPUValidateFeatures

[Jiri Denemark]

Only feature policy is checked on s390, which was previously done in
virCPUUpdate, but that's not the correct place for the check once we
have virCPUValidateFeatures.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_s390.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c
index 2ef03367d7..3d10f920ba 100644
--- a/src/cpu/cpu_s390.c
+++ b/src/cpu/cpu_s390.c
@@ -78,14 +78,6 @@ virCPUs390Update(virCPUDefPtr guest,
  goto cleanup;
 
  for (i = 0; i < guest->nfeatures; i++) {
- if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-_("only cpu feature policies 'require' and "
-  "'disable' are supported for %s"),
-guest->features[i].name);
- goto cleanup;
-}
-
 if (virCPUDefUpdateFeature(updated,
guest->features[i].name,
guest->features[i].policy) < 0)
@@ -102,6 +94,26 @@ virCPUs390Update(virCPUDefPtr guest,
  return ret;
 }
 
+
+static int
+virCPUs390ValidateFeatures(virCPUDefPtr cpu)
+{
+size_t i;
+
+for (i = 0; i < cpu->nfeatures; i++) {
+if (cpu->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("only cpu feature policies 'require' and "
+ "'disable' are supported for %s"),
+   cpu->features[i].name);
+return -1;
+}
+}
+
+return 0;
+}
+
+
 struct cpuArchDriver cpuDriverS390 = {
 .name = "s390",
 .arch = archs,
@@ -111,4 +123,5 @@ struct cpuArchDriver cpuDriverS390 = {
 .encode = NULL,
 .baseline   = NULL,
 .update = virCPUs390Update,
+.validateFeatures = virCPUs390ValidateFeatures,
 };
-- 
2.14.1

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

[libvirt] [PATCH 2/4] qemu: Validate guest CPU features before starting a domain

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7e9b406b6a..d3155e4e75 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4659,6 +4659,10 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
 if (qemuProcessStartValidateShmem(vm) < 0)
 return -1;
 
+if (vm->def->cpu &&
+virCPUValidateFeatures(vm->def->os.arch, vm->def->cpu) < 0)
+return -1;
+
 VIR_DEBUG("Checking for any possible (non-fatal) issues");
 
 qemuProcessStartWarnShmem(vm);
-- 
2.14.1

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

[libvirt] [PATCH 4/4] cpu_x86: Implement virCPUValidateFeatures

[Jiri Denemark]

The function checks whether all CPU features used in a CPU definition
are specified in cpu_map.xml.

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

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_x86.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 2864454211..5ce205f9c1 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2925,6 +2925,28 @@ virCPUx86CopyMigratable(virCPUDefPtr cpu)
 }
 
 
+static int
+virCPUx86ValidateFeatures(virCPUDefPtr cpu)
+{
+virCPUx86MapPtr map;
+size_t i;
+
+if (!(map = virCPUx86GetMap()))
+return -1;
+
+for (i = 0; i < cpu->nfeatures; i++) {
+if (!x86FeatureFind(map, cpu->features[i].name)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown CPU feature: %s"),
+   cpu->features[i].name);
+return -1;
+}
+}
+
+return 0;
+}
+
+
 int
 virCPUx86DataAddCPUID(virCPUDataPtr cpuData,
   const virCPUx86CPUID *cpuid)
@@ -3001,4 +3023,5 @@ struct cpuArchDriver cpuDriverX86 = {
 .translate  = virCPUx86Translate,
 .expandFeatures = virCPUx86ExpandFeatures,
 .copyMigratable = virCPUx86CopyMigratable,
+.validateFeatures = virCPUx86ValidateFeatures,
 };
-- 
2.14.1

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

[libvirt] [PATCH 0/4] qemu: Validate guest CPU features before starting a domain

[Jiri Denemark]

CPU features are usually checked by libvirt, but not if libvirt decides
it should not check the CPU at all, which happens with host-passthrough
CPUs, for example. Let's check all used CPU features are valid for all
CPU definitions.

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

Jiri Denemark (4):
  cpu: Introduce virCPUValidateFeatures
  qemu: Validate guest CPU features before starting a domain
  cpu_s390: Implement virCPUValidateFeatures
  cpu_x86: Implement virCPUValidateFeatures

 src/cpu/cpu.c| 29 +
 src/cpu/cpu.h|  9 +
 src/cpu/cpu_s390.c   | 29 +
 src/cpu/cpu_x86.c| 23 +++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_process.c  |  4 
 6 files changed, 87 insertions(+), 8 deletions(-)

-- 
2.14.1

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

[libvirt] [PATCH 1/4] cpu: Introduce virCPUValidateFeatures

[Jiri Denemark]

This new API may be used to check whether all features used in a CPU
definition are valid (e.g., libvirt knows their name, their policy is
supported, etc.). Leaving this API unimplemented in an arch subdriver
means libvirt does not restrict CPU features usable on the associated
architectures.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c| 29 +
 src/cpu/cpu.h|  9 +
 src/libvirt_private.syms |  1 +
 3 files changed, 39 insertions(+)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index a7c7c381b9..dc72ed42d8 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -1076,3 +1076,32 @@ virCPUCopyMigratable(virArch arch,
 else
 return virCPUDefCopy(cpu);
 }
+
+
+/**
+ * virCPUValidateFeatures:
+ *
+ * @arch: CPU architecture
+ * @cpu: CPU definition to be checked
+ *
+ * Checks whether all CPU features specified in @cpu are valid.
+ *
+ * Returns 0 on success (all features are valid), -1 on error.
+ */
+int
+virCPUValidateFeatures(virArch arch,
+   virCPUDefPtr cpu)
+{
+struct cpuArchDriver *driver;
+
+VIR_DEBUG("arch=%s, cpu=%p, nfeatures=%zu",
+  virArchToString(arch), cpu, cpu->nfeatures);
+
+if (!(driver = cpuGetSubDriver(arch)))
+return -1;
+
+if (driver->validateFeatures)
+return driver->validateFeatures(cpu);
+else
+return 0;
+}
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 5dda46ee70..5daff186c4 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -121,6 +121,9 @@ typedef int
 typedef virCPUDefPtr
 (*virCPUArchCopyMigratable)(virCPUDefPtr cpu);
 
+typedef int
+(*virCPUArchValidateFeatures)(virCPUDefPtr cpu);
+
 struct cpuArchDriver {
 const char *name;
 const virArch *arch;
@@ -142,6 +145,7 @@ struct cpuArchDriver {
 virCPUArchConvertLegacy convertLegacy;
 virCPUArchExpandFeatures expandFeatures;
 virCPUArchCopyMigratable copyMigratable;
+virCPUArchValidateFeatures validateFeatures;
 };
 
 
@@ -258,6 +262,11 @@ virCPUDefPtr
 virCPUCopyMigratable(virArch arch,
  virCPUDefPtr cpu);
 
+int
+virCPUValidateFeatures(virArch arch,
+   virCPUDefPtr cpu)
+ATTRIBUTE_NONNULL(2);
+
 /* virCPUDataFormat and virCPUDataParse are implemented for unit tests only and
  * have no real-life usage
  */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 888e4e329b..6670c5fc92 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1097,6 +1097,7 @@ virCPUProbeHost;
 virCPUTranslate;
 virCPUUpdate;
 virCPUUpdateLive;
+virCPUValidateFeatures;
 
 
 # cpu/cpu_x86.h
-- 
2.14.1

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

Re: [libvirt] [PATCH 5/8] travis: Don't have a separate script definition for macOS

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:09:05PM +0200, Andrea Bolognani wrote:
> Make single commands OS-dependent instead.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index c2526bc6d..e93fc73b2 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -77,8 +77,10 @@ before_script:
>- ./autogen.sh
>  script:
>- make -j3
> -  - make -j3 syntax-check
> -  - make -j3 check
> +  # Many unit tests still fail on macOS, and there are a bunch of issues
> +  # with syntax-check as well, so skip them for now
> +  - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check; fi
> +  - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 check; fi
>  
>  # Environments here are run in addition to the main environment defined above
>  matrix:
> @@ -91,10 +93,6 @@ matrix:
>dist: trusty
>  - compiler: clang
>os: osx
> -  script:
> -# many unit tests fail & so does syntax-check, so skip for now
> -# one day we must fix it though
> -- make -j3
>  
>  after_failure:
>- echo 
> ''
> --

Reviewed-by: Daniel P. Berrange 


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

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

Re: [libvirt] [PATCH 6/8] travis: Improve test matrix

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:09:06PM +0200, Andrea Bolognani wrote:
> The default distribution is apparently ignored if an explicit test
> matrix is provided, so we haven't actually been testing the precise
> plus gcc combo.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index e93fc73b2..154727fa3 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,8 +1,5 @@
>  sudo: false
>  language: c
> -dist: precise
> -compiler:
> -  - gcc
>  cache: ccache
>  addons:
>apt:
> @@ -82,15 +79,16 @@ script:
>- if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check; fi
>- if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 check; fi
>  
> -# Environments here are run in addition to the main environment defined above

That'll teach me to cut+paste from QEMU without checking reality :-)

>  matrix:
>include:
> +- compiler: gcc
> +  dist: precise
> +- compiler: gcc
> +  dist: trusty
>  - compiler: clang
>dist: precise
>  - compiler: clang
>dist: trusty
> -- compiler: gcc
> -  dist: trusty
>  - compiler: clang
>os: osx

Reviewed-by: Daniel P. Berrange 

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

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

Re: [libvirt] [PATCH python] Skip sparseRecvAll / sparseSendAll in sanity test

[Boris Fiuczynski]

On 09/14/2017 02:50 PM, Daniel P. Berrange wrote:

The sanity test check aims to ensure that every function listed in
the Python code maps to a corresponding C function. The Sparse
send/recv methods are special though - we're never calling the
corresponding C APIs, instead we have a pure python impl.

Signed-off-by: Daniel P. Berrange 
---
  sanitytest.py | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sanitytest.py b/sanitytest.py
index deec200..a5cb01b 100644
--- a/sanitytest.py
+++ b/sanitytest.py
@@ -351,7 +351,8 @@ for klass in gotfunctions:
  for func in sorted(gotfunctions[klass]):
  # These are pure python methods with no C APi
  if func in ["connect", "getConnect", "domain", "getDomain",
-"virEventInvokeFreeCallback"]:
+"virEventInvokeFreeCallback",
+"sparseRecvAll", "sparseSendAll"]:
  continue

  key = "%s.%s" % (klass, func)



Looks good to me.
I tested it with libvirt v3.1.0 since with libvirt-python commit id 
d7e1c97 the sanitytest failed during build.
This means all libvirt-python versions starting from 3.4.0 are hit by 
this bug.



--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCH 4/8] travis: Split building command

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:09:04PM +0200, Andrea Bolognani wrote:
> The build will fail if any of the commands fail, but this way we
> might catch more errors in a single run.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index ba8ff49a1..c2526bc6d 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -76,7 +76,9 @@ before_install:
>  before_script:
>- ./autogen.sh
>  script:
> -  - make -j3 && make -j3 syntax-check && make -j3 check
> +  - make -j3
> +  - make -j3 syntax-check
> +  - make -j3 check
>  
>  # Environments here are run in addition to the main environment defined above
>  matrix:

The downside of this is that if syntax-check fails, but check succeeeds,
you now have to search through the middle of the logfile to find the
failure, instead of just jumping straight to the end.

So I'm on the fence about this patch - I'd have a slight preference for
existing behaviour of failing fast to keep errors at the end of the log

None the less

Reviewed-by: Daniel P. Berrange 

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

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

Re: [libvirt] [PATCH 7/8] travis: Install more build dependencies

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:09:07PM +0200, Andrea Bolognani wrote:
> Since configure automatically picks up as many optional dependencies
> as possible, installing more packages allows us to improve our test
> coverage.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 37 ++---
>  1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 154727fa3..f56e6ae6f 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -5,37 +5,60 @@ addons:
>apt:
>  # Please keep this list sorted alphabetically
>  packages:
> +  - autoconf
> +  - automake
>- autopoint
> +  - ccache
>- dnsmasq-base
> -# - dwarves
> +  - ebtables
> +  - gcc
>- gettext
> +  - glusterfs-client
> +  - libacl1-dev
>- libapparmor-dev
> +  - libattr1-dev
>- libaudit-dev
>- libavahi-client-dev
> +  - libblkid-dev
> +  - libc6-dev
>- libcap-ng-dev
> +  - libcurl4-gnutls-dev
> +  - libdbus-1-dev
>- libdevmapper-dev
> -  - libgcrypt11-dev
> +  - libfuse-dev
>- libgnutls-dev
> -  - libncurses5-dev
>- libnetcf-dev
>- libnl-3-dev
>- libnl-route-3-dev
>- libnuma-dev
> -  - libparted0-dev
> -  - libpcap0.8-dev
> +  - libopenwsman-dev
> +  - libparted-dev
> +  - libpcap-dev
>- libpciaccess-dev
>- librbd-dev
>- libreadline-dev
>- libsasl2-dev
> +  - libssh2-1-dev
> +  - libssl-dev
> +  - libtool
>- libudev-dev
>- libxen-dev
>- libxml2-dev
>- libxml2-utils
>- libyajl-dev
>- lvm2
> -  - uuid-dev
> +  - make
> +  - nfs-common
> +  - open-iscsi
> +  - parted
> +  - patch
> +  - pkg-config
> +  - policykit-1
> +  - scrub
> +  - sheepdog
> +  - systemtap-sdt-dev
>- xsltproc
> -  - zlib1g-dev
> +  - zfs-fuse
>  
>  notifications:
>irc:

Did you check if travis actually lets you install all these ?  They have
to whitelist each individual package that's desired. So if some of these
aren't in the whitelist, adding them here will have no effect IIUC.


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 3/8] travis: Don't abort build due to -Wvariadic-macros

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:09:03PM +0200, Andrea Bolognani wrote:
> The openwsman header files are at fault here, but precise is entirely
> unmaintained at this point so the issue will never be fixed.
> 
> Better to ignore the error and have coverage over the Hyper-V driver
> than disabling it: if code that would trigger the warning will be
> added to libvirt, the CentOS CI will catch it.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 367baf861..ba8ff49a1 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -65,6 +65,9 @@ env:
>  # The custom $PATH is just to pick up some extra binaries installed
>  # through homebrew on macOS and it's completely harmless on Linux
>  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH"
> +# The hyperv driver fails to build with clang on precise due to this
> +# error being raised in one of openwsman header files
> +- CFLAGS="-Wno-error=variadic-macros"
>  - VIR_TEST_DEBUG=1
>  
>  before_install:

Reviewed-by: Daniel P. Berrange 

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

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

Re: [libvirt] [PATCH 2/8] travis: Move variables to 'env' section

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:09:02PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 8831f742c..367baf861 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -60,14 +60,20 @@ git:
>depth: 5
>submodules: true
>  
> +env:
> +  global:
> +# The custom $PATH is just to pick up some extra binaries installed
> +# through homebrew on macOS and it's completely harmless on Linux
> +- PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH"
> +- VIR_TEST_DEBUG=1
> +
>  before_install:
>- if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install 
> gnutls libgcrypt yajl gettext rpcgen ; fi
>  
> -# the custom PATH is just to pick up OS-X homebrew & its harmless on Linux
>  before_script:
> -  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
> ./autogen.sh
> +  - ./autogen.sh
>  script:
> -  - VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
> +  - make -j3 && make -j3 syntax-check && make -j3 check
>  
>  # Environments here are run in addition to the main environment defined above
>  matrix:

Reviewed-by: Daniel P. Berrange 

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

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

Re: [libvirt] [PATCH 1/8] travis: Limit git depth to 5 commits

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:09:01PM +0200, Andrea Bolognani wrote:
> We don't need 50 commits for our purposes, so might as well save some
> bandwidth and possibly some time by making the clone more shallow.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.travis.yml b/.travis.yml
> index f59bd19c8..8831f742c 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -57,6 +57,7 @@ notifications:
>- secure: 
> "QcU9eP96P0RlDNzVRZl/4sxyydPStGzECrpgJhr2IPB/7pHk23yaBrmUsq9S830tB+jwLGma1IscNB8uf7Sf7WY+cYIpfR8v030OffWnaipo/Gcs0dpnlfURWHjOFQI3RJzGEihsqvbwUFOwsM+3IDyO3qdWaiT6cN2Tj9ROlwYCySSX5YWzLyX7arBZ4lp8ESs7ohQaEwp2cegnMP2oGPJJe4SebvlCDjHZbjkU5aEradwUWnRQDJZWTKknpNLArVFxN2/ixp6f/MGY4DmkHoDweio6mHIPN5zTs5Jt32aiX6wDBa+bBa4v8TCRqzhYkQ63ZZhNV8bY5Uf9ufTdyvt96yIANyakd85b1QpMdAX76IyJi1l0/Uub6DTQZAcq3vK7iPjGeTVSpyoXrqTfGy4JxMjqDoocpWvv8ALX1wrYI/HfN2R2Aepw9jModTimOsebYhJ1yMhSt8qnh5AQNftGKL2JBKoA1LWdU2YJ5fO1bGjKNiVEkGFQTPYFWrYCUY5JcT+s5WCzNeMNm8s9na8liYhGl3WtS3rPr5M8bof+BMsBhG2hQ0loduc94x2GkvyhQZUgRbqrwNR+y4hn+rWFC3hBzzyiAULs43vY/PJ+eBdKEf3VAc0MkhQ8GgXGSA61fR6aXYonroI/WnBVItwDmUnnMfSziZXxk09GLl4="
>  
>  git:
> +  depth: 5
>submodules: true
>  
>  before_install:

Reviewed-by: Daniel P. Berrange 


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

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

[libvirt] [PATCH 5/8] travis: Don't have a separate script definition for macOS

[Andrea Bolognani]

Make single commands OS-dependent instead.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index c2526bc6d..e93fc73b2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -77,8 +77,10 @@ before_script:
   - ./autogen.sh
 script:
   - make -j3
-  - make -j3 syntax-check
-  - make -j3 check
+  # Many unit tests still fail on macOS, and there are a bunch of issues
+  # with syntax-check as well, so skip them for now
+  - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check; fi
+  - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 check; fi
 
 # Environments here are run in addition to the main environment defined above
 matrix:
@@ -91,10 +93,6 @@ matrix:
   dist: trusty
 - compiler: clang
   os: osx
-  script:
-# many unit tests fail & so does syntax-check, so skip for now
-# one day we must fix it though
-- make -j3
 
 after_failure:
   - echo 
''
-- 
2.13.5

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

[libvirt] [PATCH 2/8] travis: Move variables to 'env' section

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 8831f742c..367baf861 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -60,14 +60,20 @@ git:
   depth: 5
   submodules: true
 
+env:
+  global:
+# The custom $PATH is just to pick up some extra binaries installed
+# through homebrew on macOS and it's completely harmless on Linux
+- PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH"
+- VIR_TEST_DEBUG=1
+
 before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install gnutls 
libgcrypt yajl gettext rpcgen ; fi
 
-# the custom PATH is just to pick up OS-X homebrew & its harmless on Linux
 before_script:
-  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
./autogen.sh
+  - ./autogen.sh
 script:
-  - VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
+  - make -j3 && make -j3 syntax-check && make -j3 check
 
 # Environments here are run in addition to the main environment defined above
 matrix:
-- 
2.13.5

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

[libvirt] [PATCH 3/8] travis: Don't abort build due to -Wvariadic-macros

[Andrea Bolognani]

The openwsman header files are at fault here, but precise is entirely
unmaintained at this point so the issue will never be fixed.

Better to ignore the error and have coverage over the Hyper-V driver
than disabling it: if code that would trigger the warning will be
added to libvirt, the CentOS CI will catch it.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 367baf861..ba8ff49a1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -65,6 +65,9 @@ env:
 # The custom $PATH is just to pick up some extra binaries installed
 # through homebrew on macOS and it's completely harmless on Linux
 - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH"
+# The hyperv driver fails to build with clang on precise due to this
+# error being raised in one of openwsman header files
+- CFLAGS="-Wno-error=variadic-macros"
 - VIR_TEST_DEBUG=1
 
 before_install:
-- 
2.13.5

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

[libvirt] [PATCH 4/8] travis: Split building command

[Andrea Bolognani]

The build will fail if any of the commands fail, but this way we
might catch more errors in a single run.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index ba8ff49a1..c2526bc6d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -76,7 +76,9 @@ before_install:
 before_script:
   - ./autogen.sh
 script:
-  - make -j3 && make -j3 syntax-check && make -j3 check
+  - make -j3
+  - make -j3 syntax-check
+  - make -j3 check
 
 # Environments here are run in addition to the main environment defined above
 matrix:
-- 
2.13.5

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

Re: [libvirt] [libvirt-python][PATCH] maint: Require libvirt-3.4.0 at least

[Michal Privoznik]

On 09/14/2017 02:42 PM, Daniel P. Berrange wrote:
> On Thu, Sep 14, 2017 at 01:41:08PM +0200, Michal Privoznik wrote:
>> Currently, we require 0.9.11. However, some APIs are missing
>> there and thus sanity check fails:
>>
>> DEBUG: /usr/bin/python sanitytest.py build/lib.linux-s390x-2.7 
>> /usr/share/libvirt/api/libvirt-api.xml
>> DEBUG: FAIL virStream.sparseRecvAll   (Python API not mapped to C)
>> DEBUG: FAIL virStream.sparseSendAll   (Python API not mapped to C)
>> DEBUG: error: command '/usr/bin/python' failed with exit status 1
>>
>> I'm not sure how to fix that so raising minimal required libvirt
>> version is the solution.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>  setup.py   |   2 +-
>>  README |   2 +-
>>  libvirt-override.c | 149 
>> -
>>  3 files changed, 2 insertions(+), 151 deletions(-)
>>
>> diff --git a/setup.py b/setup.py
>> index f33ff1a..f929eb2 100755
>> --- a/setup.py
>> +++ b/setup.py
>> @@ -17,7 +17,7 @@ import re
>>  import shutil
>>  import time
>>  
>> -MIN_LIBVIRT = "0.9.11"
>> +MIN_LIBVIRT = "3.4.0"
> 
> NACK, we cannot do this - it will break many people and apps (OpenStack
> in particular) who expect latest libvirt on pypi to work with historical
> C libs.

I don't know how pypi works, but if somebody distributes just
libvirt-python and doesn't ship libvirt.so too, such process is broken
already because libvirt-python could have been compiled with one version
of libvirt while user might be running a different one. So shipping
libvirt.so is the only way. And since libvirt-python doesn't add any new
features compared to bare libvirt, why on earth would somebody want to
run latest libvirt-python but an ancient libvirt? It doesn't make much
sense to update one without other.

Michal

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

[libvirt] [PATCH 7/8] travis: Install more build dependencies

[Andrea Bolognani]

Since configure automatically picks up as many optional dependencies
as possible, installing more packages allows us to improve our test
coverage.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 37 ++---
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 154727fa3..f56e6ae6f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,37 +5,60 @@ addons:
   apt:
 # Please keep this list sorted alphabetically
 packages:
+  - autoconf
+  - automake
   - autopoint
+  - ccache
   - dnsmasq-base
-# - dwarves
+  - ebtables
+  - gcc
   - gettext
+  - glusterfs-client
+  - libacl1-dev
   - libapparmor-dev
+  - libattr1-dev
   - libaudit-dev
   - libavahi-client-dev
+  - libblkid-dev
+  - libc6-dev
   - libcap-ng-dev
+  - libcurl4-gnutls-dev
+  - libdbus-1-dev
   - libdevmapper-dev
-  - libgcrypt11-dev
+  - libfuse-dev
   - libgnutls-dev
-  - libncurses5-dev
   - libnetcf-dev
   - libnl-3-dev
   - libnl-route-3-dev
   - libnuma-dev
-  - libparted0-dev
-  - libpcap0.8-dev
+  - libopenwsman-dev
+  - libparted-dev
+  - libpcap-dev
   - libpciaccess-dev
   - librbd-dev
   - libreadline-dev
   - libsasl2-dev
+  - libssh2-1-dev
+  - libssl-dev
+  - libtool
   - libudev-dev
   - libxen-dev
   - libxml2-dev
   - libxml2-utils
   - libyajl-dev
   - lvm2
-  - uuid-dev
+  - make
+  - nfs-common
+  - open-iscsi
+  - parted
+  - patch
+  - pkg-config
+  - policykit-1
+  - scrub
+  - sheepdog
+  - systemtap-sdt-dev
   - xsltproc
-  - zlib1g-dev
+  - zfs-fuse
 
 notifications:
   irc:
-- 
2.13.5

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

[libvirt] [PATCH 1/8] travis: Limit git depth to 5 commits

[Andrea Bolognani]

We don't need 50 commits for our purposes, so might as well save some
bandwidth and possibly some time by making the clone more shallow.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index f59bd19c8..8831f742c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -57,6 +57,7 @@ notifications:
   - secure: 
"QcU9eP96P0RlDNzVRZl/4sxyydPStGzECrpgJhr2IPB/7pHk23yaBrmUsq9S830tB+jwLGma1IscNB8uf7Sf7WY+cYIpfR8v030OffWnaipo/Gcs0dpnlfURWHjOFQI3RJzGEihsqvbwUFOwsM+3IDyO3qdWaiT6cN2Tj9ROlwYCySSX5YWzLyX7arBZ4lp8ESs7ohQaEwp2cegnMP2oGPJJe4SebvlCDjHZbjkU5aEradwUWnRQDJZWTKknpNLArVFxN2/ixp6f/MGY4DmkHoDweio6mHIPN5zTs5Jt32aiX6wDBa+bBa4v8TCRqzhYkQ63ZZhNV8bY5Uf9ufTdyvt96yIANyakd85b1QpMdAX76IyJi1l0/Uub6DTQZAcq3vK7iPjGeTVSpyoXrqTfGy4JxMjqDoocpWvv8ALX1wrYI/HfN2R2Aepw9jModTimOsebYhJ1yMhSt8qnh5AQNftGKL2JBKoA1LWdU2YJ5fO1bGjKNiVEkGFQTPYFWrYCUY5JcT+s5WCzNeMNm8s9na8liYhGl3WtS3rPr5M8bof+BMsBhG2hQ0loduc94x2GkvyhQZUgRbqrwNR+y4hn+rWFC3hBzzyiAULs43vY/PJ+eBdKEf3VAc0MkhQ8GgXGSA61fR6aXYonroI/WnBVItwDmUnnMfSziZXxk09GLl4="
 
 git:
+  depth: 5
   submodules: true
 
 before_install:
-- 
2.13.5

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

Re: [libvirt] [libvirt-python][PATCH] maint: Require libvirt-3.4.0 at least

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 03:05:15PM +0200, Michal Privoznik wrote:
> On 09/14/2017 02:42 PM, Daniel P. Berrange wrote:
> > On Thu, Sep 14, 2017 at 01:41:08PM +0200, Michal Privoznik wrote:
> >> Currently, we require 0.9.11. However, some APIs are missing
> >> there and thus sanity check fails:
> >>
> >> DEBUG: /usr/bin/python sanitytest.py build/lib.linux-s390x-2.7 
> >> /usr/share/libvirt/api/libvirt-api.xml
> >> DEBUG: FAIL virStream.sparseRecvAll   (Python API not mapped to C)
> >> DEBUG: FAIL virStream.sparseSendAll   (Python API not mapped to C)
> >> DEBUG: error: command '/usr/bin/python' failed with exit status 1
> >>
> >> I'm not sure how to fix that so raising minimal required libvirt
> >> version is the solution.
> >>
> >> Signed-off-by: Michal Privoznik 
> >> ---
> >>  setup.py   |   2 +-
> >>  README |   2 +-
> >>  libvirt-override.c | 149 
> >> -
> >>  3 files changed, 2 insertions(+), 151 deletions(-)
> >>
> >> diff --git a/setup.py b/setup.py
> >> index f33ff1a..f929eb2 100755
> >> --- a/setup.py
> >> +++ b/setup.py
> >> @@ -17,7 +17,7 @@ import re
> >>  import shutil
> >>  import time
> >>  
> >> -MIN_LIBVIRT = "0.9.11"
> >> +MIN_LIBVIRT = "3.4.0"
> > 
> > NACK, we cannot do this - it will break many people and apps (OpenStack
> > in particular) who expect latest libvirt on pypi to work with historical
> > C libs.
> 
> I don't know how pypi works, but if somebody distributes just
> libvirt-python and doesn't ship libvirt.so too, such process is broken
> already because libvirt-python could have been compiled with one version
> of libvirt while user might be running a different one. So shipping
> libvirt.so is the only way. And since libvirt-python doesn't add any new
> features compared to bare libvirt, why on earth would somebody want to
> run latest libvirt-python but an ancient libvirt? It doesn't make much
> sense to update one without other.

Pypi only ships the source tar.gz. When you pip install libvirt-python
it is then built against the libvirt C library you have installed. The
newer libvirt-python bindings sometimes fix bugs wrt bindings of previously
existing APIs, so there is a reason to run newer libvirt-python. In addition
building libvirt-python from pypi instead of relying on distro installed
RPMs of libvirt-python lets apps use arbitrary versions of python, not just
the one the distro built against. This all works very well and is relied
upon by OpenStack.

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

[libvirt] [PATCH 0/8] travis: Fix and improve configuration

[Andrea Bolognani]

This series fixes the precise+gcc combo not being actually tested
and significantly improves test coverage by installing a bunch of
additional optional dependencies on the workers.

Andrea Bolognani (8):
  travis: Limit git depth to 5 commits
  travis: Move variables to 'env' section
  travis: Don't abort build due to -Wvariadic-macros
  travis: Split building command
  travis: Don't have a separate script definition for macOS
  travis: Improve test matrix
  travis: Install more build dependencies
  travis: Shuffle sections around

 .travis.yml | 125 ++--
 1 file changed, 79 insertions(+), 46 deletions(-)

-- 
2.13.5

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

Re: [libvirt] [libvirt-python][PATCH] maint: Require libvirt-3.4.0 at least

[Martin Kletzander]

On Thu, Sep 14, 2017 at 01:41:08PM +0200, Michal Privoznik wrote:

Currently, we require 0.9.11. However, some APIs are missing
there and thus sanity check fails:

DEBUG: /usr/bin/python sanitytest.py build/lib.linux-s390x-2.7 
/usr/share/libvirt/api/libvirt-api.xml
DEBUG: FAIL virStream.sparseRecvAll   (Python API not mapped to C)
DEBUG: FAIL virStream.sparseSendAll   (Python API not mapped to C)
DEBUG: error: command '/usr/bin/python' failed with exit status 1

I'm not sure how to fix that so raising minimal required libvirt
version is the solution.



I think the whole point is to be able to compile with any older (sane)
version of libvirt, so you should *not* do this.  We already have a
mechanism for this, it's just that the naming of these functions doesn't
fit it and it needs to be adjusted IIRC.  See function shouldSkip() in
generator.py.

From me this is NACK until convinced otherwise.


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

[libvirt] [PATCH 8/8] travis: Shuffle sections around

[Andrea Bolognani]

Order them more logically and make sure that stuff that doesn't
need to be modified frequently if at all, such as the notification
settings, are out of the way.

Perform other minor tweaks as well.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 62 +++--
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index f56e6ae6f..f9ee36c17 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,20 @@
 sudo: false
 language: c
 cache: ccache
+
+matrix:
+  include:
+- compiler: gcc
+  dist: precise
+- compiler: gcc
+  dist: trusty
+- compiler: clang
+  dist: precise
+- compiler: clang
+  dist: trusty
+- compiler: clang
+  os: osx
+
 addons:
   apt:
 # Please keep this list sorted alphabetically
@@ -60,22 +74,6 @@ addons:
   - xsltproc
   - zfs-fuse
 
-notifications:
-  irc:
-# The channel name "irc.oftc.net#virt" is encrypted against libvirt/libvirt
-# to prevent IRC notifications from github forks. This was created using:
-# $ travis encrypt -r "libvirt/libvirt" "irc.oftc.net#virt"
-channels:
-  - secure: 
"hUPdkLxX7nh75+clpnk4U0XLExLfV9DFKSvQSAUtf5JtDNMslj7AeOCf2wcbkNsEhkiF557odTAnov1s5m1w/yaa56zbjFAh5agzqRKya3QjqsrvlBKw/WuN+l82iMNLLeebTgCPAXrbAbGWH8YmYssp/7+eMsnKaVh84EQQNbMCHlLg6ovE26Fs18mZ6J5RC3OPa1vbv+xkdCHvGg/Oyp4K8bpU7RYyimA56jdxI/OfdTH9HxntHYSzykR7hDbyzZhdIlAUyRKReQVjcV5+R8fdDL/1imyGA/88KTztMeKXpZ5Rf+Ss3vYLZb6qsLLegCZ4AU/q0vvbWxjpZGJZoeyrVpfBTZdYGIzmLTMl9GYXXa/gDwFlbvRDiPDG4TIy6GlMUROinj7KRKEHu1fWRYu012ife5OjidxcwrTnz21vYaCv3AKWPpMPxwIzQPkY1hex9uLLX6z+TrAxxDLF+7UzRT9w2RLFBkLYlj2aDVrLAVb/ynRsxDz5CGzC61FSQVft2e308SkGjdn8YxvguCuXv+N70Fu1cvFyh5XYeHb4fbBRo0Ctzaec78leHlQvRGWKJxXDXRkE2lvvBc7YbBNSAYh7Fs8Y+zY7l7rMxvXdrt3nuaNQhe74V3yhxPDAld66qmAn9TYMmaZW2f5/KKKILLbCa0t2MxiAc6L2OI8="
-on_success: change
-on_failure: always
-  email:
-# The list name 'libvirt...@redhat.com" is encrypted against 
libvirt/libvirt
-# to prevent IRC notifications from github forks. This was created using:
-# $ travis encrypt -r "libvirt/libvirt" "libvirt...@redhat.com"
-recipients:
-  - secure: 
"QcU9eP96P0RlDNzVRZl/4sxyydPStGzECrpgJhr2IPB/7pHk23yaBrmUsq9S830tB+jwLGma1IscNB8uf7Sf7WY+cYIpfR8v030OffWnaipo/Gcs0dpnlfURWHjOFQI3RJzGEihsqvbwUFOwsM+3IDyO3qdWaiT6cN2Tj9ROlwYCySSX5YWzLyX7arBZ4lp8ESs7ohQaEwp2cegnMP2oGPJJe4SebvlCDjHZbjkU5aEradwUWnRQDJZWTKknpNLArVFxN2/ixp6f/MGY4DmkHoDweio6mHIPN5zTs5Jt32aiX6wDBa+bBa4v8TCRqzhYkQ63ZZhNV8bY5Uf9ufTdyvt96yIANyakd85b1QpMdAX76IyJi1l0/Uub6DTQZAcq3vK7iPjGeTVSpyoXrqTfGy4JxMjqDoocpWvv8ALX1wrYI/HfN2R2Aepw9jModTimOsebYhJ1yMhSt8qnh5AQNftGKL2JBKoA1LWdU2YJ5fO1bGjKNiVEkGFQTPYFWrYCUY5JcT+s5WCzNeMNm8s9na8liYhGl3WtS3rPr5M8bof+BMsBhG2hQ0loduc94x2GkvyhQZUgRbqrwNR+y4hn+rWFC3hBzzyiAULs43vY/PJ+eBdKEf3VAc0MkhQ8GgXGSA61fR6aXYonroI/WnBVItwDmUnnMfSziZXxk09GLl4="
-
 git:
   depth: 5
   submodules: true
@@ -91,10 +89,11 @@ env:
 - VIR_TEST_DEBUG=1
 
 before_install:
-  - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install gnutls 
libgcrypt yajl gettext rpcgen ; fi
+  - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install gnutls 
libgcrypt yajl gettext rpcgen; fi
 
 before_script:
   - ./autogen.sh
+
 script:
   - make -j3
   # Many unit tests still fail on macOS, and there are a bunch of issues
@@ -102,19 +101,6 @@ script:
   - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check; fi
   - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 check; fi
 
-matrix:
-  include:
-- compiler: gcc
-  dist: precise
-- compiler: gcc
-  dist: trusty
-- compiler: clang
-  dist: precise
-- compiler: clang
-  dist: trusty
-- compiler: clang
-  os: osx
-
 after_failure:
   - echo 
''
   - 'if [ -f $(pwd)/tests/test-suite.log ]; then
@@ -122,3 +108,19 @@ after_failure:
 else
 echo "=== NO LOG FILE FOUND ===";
 fi'
+
+notifications:
+  irc:
+# The channel name "irc.oftc.net#virt" is encrypted against libvirt/libvirt
+# to prevent IRC notifications from github forks. This was created using:
+# $ travis encrypt -r "libvirt/libvirt" "irc.oftc.net#virt"
+channels:
+  - secure: 
"hUPdkLxX7nh75+clpnk4U0XLExLfV9DFKSvQSAUtf5JtDNMslj7AeOCf2wcbkNsEhkiF557odTAnov1s5m1w/yaa56zbjFAh5agzqRKya3QjqsrvlBKw/WuN+l82iMNLLeebTgCPAXrbAbGWH8YmYssp/7+eMsnKaVh84EQQNbMCHlLg6ovE26Fs18mZ6J5RC3OPa1vbv+xkdCHvGg/Oyp4K8bpU7RYyimA56jdxI/OfdTH9HxntHYSzykR7hDbyzZhdIlAUyRKReQVjcV5+R8fdDL/1imyGA/88KTztMeKXpZ5Rf+Ss3vYLZb6qsLLegCZ4AU/q0vvbWxjpZGJZoeyrVpfBTZdYGIzmLTMl9GYXXa/gDwFlbvRDiPDG4TIy6GlMUROinj7KRKEHu1fWRYu012ife5OjidxcwrTnz21vYaCv3AKWPpMPxwIzQPkY1hex9uLLX6z+TrAxxDLF+7UzRT9w2RLFBkLYlj2aDVrLAVb/ynRsxDz5CGzC61FSQVft2e308SkGjdn8YxvguCuXv+N70Fu1cvFyh5XYeHb4fbBRo0Ctzaec78leHlQvRGWKJxXDXRkE2lvvBc7YbBNSAYh7Fs8Y+zY7l7rMxvXdrt3nuaNQhe74V3yhxPDAld66qmAn9TYMmaZW2f5/KKKILLbCa0t2MxiAc6L2OI8="

[libvirt] [PATCH 6/8] travis: Improve test matrix

[Andrea Bolognani]

The default distribution is apparently ignored if an explicit test
matrix is provided, so we haven't actually been testing the precise
plus gcc combo.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index e93fc73b2..154727fa3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,8 +1,5 @@
 sudo: false
 language: c
-dist: precise
-compiler:
-  - gcc
 cache: ccache
 addons:
   apt:
@@ -82,15 +79,16 @@ script:
   - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 syntax-check; fi
   - if [ "$TRAVIS_OS_NAME" != "osx" ]; then make -j3 check; fi
 
-# Environments here are run in addition to the main environment defined above
 matrix:
   include:
+- compiler: gcc
+  dist: precise
+- compiler: gcc
+  dist: trusty
 - compiler: clang
   dist: precise
 - compiler: clang
   dist: trusty
-- compiler: gcc
-  dist: trusty
 - compiler: clang
   os: osx
 
-- 
2.13.5

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

[libvirt] [PATCH v8 05/11] qemu: Refactor qemuBlockStorageSourceBuildHostsJSONSocketAddress

[John Ferlan]

From: Ashish Mittal 

Extract out the "guts" of building a server entry into it's own
separately callable/usable function in order to allow building
a server entry for a consumer with src->nhosts == 1.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_block.c | 106 +-
 1 file changed, 70 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index d07269f4e..c97b787c5 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -380,6 +380,74 @@ qemuBlockGetNodeData(virJSONValuePtr data)
 
 
 /**
+ * qemuBlockStorageSourceBuildJSONSocketAddress
+ * @host: the virStorageNetHostDefPtr definition to build
+ * @legacy: use 'tcp' instead of 'inet' for compatibility reasons
+ *
+ * Formats @hosts into a json object conforming to the 'SocketAddress' type
+ * in qemu.
+ *
+ * This function can be used when only 1 src->nhosts is expected in order
+ * to build a command without the array indices after "server.". That is
+ * to see "server.type", "server.host", and "server.port" instead of
+ * "server.#.type", "server.#.host", and "server.#.port".
+ *
+ * Returns a virJSONValuePtr for a single server.
+ */
+static virJSONValuePtr
+qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host,
+ bool legacy)
+{
+virJSONValuePtr server = NULL;
+virJSONValuePtr ret = NULL;
+const char *transport;
+char *port = NULL;
+
+switch ((virStorageNetHostTransport) host->transport) {
+case VIR_STORAGE_NET_HOST_TRANS_TCP:
+if (legacy)
+transport = "tcp";
+else
+transport = "inet";
+
+if (virAsprintf(, "%u", host->port) < 0)
+goto cleanup;
+
+if (virJSONValueObjectCreate(,
+ "s:type", transport,
+ "s:host", host->name,
+ "s:port", port,
+ NULL) < 0)
+goto cleanup;
+break;
+
+case VIR_STORAGE_NET_HOST_TRANS_UNIX:
+if (virJSONValueObjectCreate(,
+ "s:type", "unix",
+ "s:socket", host->socket,
+ NULL) < 0)
+goto cleanup;
+break;
+
+case VIR_STORAGE_NET_HOST_TRANS_RDMA:
+case VIR_STORAGE_NET_HOST_TRANS_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("transport protocol '%s' is not yet supported"),
+   
virStorageNetHostTransportTypeToString(host->transport));
+goto cleanup;
+}
+
+VIR_STEAL_PTR(ret, server);
+
+ cleanup:
+VIR_FREE(port);
+virJSONValueFree(server);
+
+return ret;
+}
+
+
+/**
  * qemuBlockStorageSourceBuildHostsJSONSocketAddress:
  * @src: disk storage source
  * @legacy: use 'tcp' instead of 'inet' for compatibility reasons
@@ -395,8 +463,6 @@ 
qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src,
 virJSONValuePtr server = NULL;
 virJSONValuePtr ret = NULL;
 virStorageNetHostDefPtr host;
-const char *transport;
-char *port = NULL;
 size_t i;
 
 if (!(servers = virJSONValueNewArray()))
@@ -405,39 +471,8 @@ 
qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src,
 for (i = 0; i < src->nhosts; i++) {
 host = src->hosts + i;
 
-switch ((virStorageNetHostTransport) host->transport) {
-case VIR_STORAGE_NET_HOST_TRANS_TCP:
-if (legacy)
-transport = "tcp";
-else
-transport = "inet";
-
-if (virAsprintf(, "%u", host->port) < 0)
-goto cleanup;
-
-if (virJSONValueObjectCreate(,
- "s:type", transport,
- "s:host", host->name,
- "s:port", port,
- NULL) < 0)
-goto cleanup;
-break;
-
-case VIR_STORAGE_NET_HOST_TRANS_UNIX:
-if (virJSONValueObjectCreate(,
- "s:type", "unix",
- "s:socket", host->socket,
- NULL) < 0)
-goto cleanup;
-break;
-
-case VIR_STORAGE_NET_HOST_TRANS_RDMA:
-case VIR_STORAGE_NET_HOST_TRANS_LAST:
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("transport protocol '%s' is not yet supported"),
-   
virStorageNetHostTransportTypeToString(host->transport));
-goto cleanup;
-}
+if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(host, 
legacy)))
+  goto cleanup;
 
 if 

[libvirt] [PATCH v8 03/11] docs: Add schema and docs for Veritas HyperScale (VxHS)

[John Ferlan]

From: Ashish Mittal 

Alter the schema to allow a VxHS block device. Sample XML is:

  


  


eb90327c-8302-4725-9e1b-4e85ed4dc251

  

Update the html docs to describe the capability for VxHS.

Alter the qemuxml2xmltest to validate the formatting.

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 docs/formatdomain.html.in  | 15 --
 docs/schemas/domaincommon.rng  | 13 +
 .../qemuxml2argv-disk-drive-network-vxhs.xml   | 32 
 .../qemuxml2xmlout-disk-drive-network-vxhs.xml | 34 ++
 tests/qemuxml2xmltest.c|  1 +
 5 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-vxhs.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8ca7637a4..4464c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2520,9 +2520,9 @@
   
   The protocol attribute specifies the protocol to
   access to the requested image. Possible values are "nbd",
-  "iscsi", "rbd", "sheepdog" or "gluster".  If the
-  protocol attribute is "rbd", "sheepdog" or
-  "gluster", an additional attribute name is
+  "iscsi", "rbd", "sheepdog", "gluster" or "vxhs".  If the
+  protocol attribute is "rbd", "sheepdog", "gluster"
+  or "vxhs", an additional attribute name is
   mandatory to specify which volume/image will be used. For "nbd",
   the name attribute is optional. For "iscsi"
   (since 1.0.4), the name
@@ -2530,6 +2530,9 @@
   target's name by a slash (e.g.,
   iqn.2013-07.com.example:iscsi-pool/1). If not
   specified, the default LUN is zero.
+  For "vxhs" (since 3.8.0), the
+  name is the UUID of the volume, assigned by the
+  HyperScale server.
   Since 0.8.7
   
 volume
@@ -2632,6 +2635,12 @@
  one or more (Since 2.1.0), 
just one prior to that 
  24007 
   
+  
+ vxhs 
+ a server running Veritas HyperScale daemon 
+ only one 
+  
+  
 
 
 gluster supports "tcp", "rdma", "unix" as valid values for the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c9a4f7a9a..76852abb3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1636,6 +1636,18 @@
 
   
 
+  
+
+  
+
+  vxhs
+
+  
+  
+  
+
+  
+
   
 
   network
@@ -1646,6 +1658,7 @@
   
   
   
+  
 
   
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml
new file mode 100644
index 0..4f4df2f9e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml
@@ -0,0 +1,32 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+
+  
+  
+  eb90327c-8302-4725-9e1b-4e85ed4dc251
+  
+
+
+
+
+
+
+  
+
diff --git 
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-vxhs.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-vxhs.xml
new file mode 100644
index 0..160ed8d5f
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-vxhs.xml
@@ -0,0 +1,34 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+
+  
+  
+  eb90327c-8302-4725-9e1b-4e85ed4dc251
+  
+
+
+  
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0a87cedf2..8b7577fd3 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -474,6 +474,7 @@ mymain(void)
 DO_TEST("disk-drive-network-rbd-ipv6", NONE);
 DO_TEST("disk-drive-network-rbd-ceph-env", NONE);
 DO_TEST("disk-drive-network-sheepdog", NONE);
+DO_TEST("disk-drive-network-vxhs", NONE);
 DO_TEST("disk-scsi-device",
 QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI);
 DO_TEST("disk-scsi-vscsi", NONE);
-- 
2.13.5

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

[libvirt] [PATCH v8 02/11] storage: Introduce VIR_STORAGE_NET_PROTOCOL_VXHS

[John Ferlan]

From: Ashish Mittal 

Add a new virStorageNetProtocol for Veritas HyperScale (VxHS) disks

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 src/libxl/libxl_conf.c| 1 +
 src/qemu/qemu_block.c | 1 +
 src/qemu/qemu_command.c   | 1 +
 src/qemu/qemu_driver.c| 3 +++
 src/qemu/qemu_parse_command.c | 1 +
 src/util/virstoragefile.c | 5 -
 src/util/virstoragefile.h | 1 +
 src/xenconfig/xen_xl.c| 1 +
 8 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4416a09dd..34233a955 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -666,6 +666,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src,
 case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
 case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 virReportError(VIR_ERR_NO_SUPPORT,
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 7fb12ea5a..d07269f4e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -522,6 +522,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr 
src)
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 break;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d553df57f..720530daa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -999,6 +999,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
_("'ssh' protocol is not yet supported"));
 goto cleanup;
 
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b334cf20b..d299938c5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13838,6 +13838,7 @@ 
qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("external inactive snapshots are not supported on 
"
@@ -13901,6 +13902,7 @@ 
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("external active snapshots are not supported on "
@@ -14046,6 +14048,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr 
conn,
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("internal inactive snapshots are not supported on 
"
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 8cb96a24a..9190a37ba 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -2026,6 +2026,7 @@ qemuParseCommandLine(virCapsPtr caps,
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 /* ignored for now */
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index e94ad32f0..ca306c27b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -85,7 +85,8 @@ VIR_ENUM_IMPL(virStorageNetProtocol, 
VIR_STORAGE_NET_PROTOCOL_LAST,
   "ftp",
   "ftps",
   "tftp",
-  "ssh")
+  "ssh",
+  "vxhs")
 
 VIR_ENUM_IMPL(virStorageNetHostTransport, VIR_STORAGE_NET_HOST_TRANS_LAST,
   "tcp",
@@ -2712,6 +2713,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src,
 case VIR_STORAGE_NET_PROTOCOL_ISCSI:
 case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
  

[libvirt] [PATCH v8 01/11] qemu: Detect support for vxhs

[John Ferlan]

Using the query-qmp-schema introspection - look for the 'vxhs'
blockdevOptions type.

NB: This is a "best effort" type situation as there is not a
mechanism to determine whether the running QEMU has been
built with '--enable-vxhs'. All we can do is check if the
option to use vxhs for a blockdev-add exists in the command
infrastructure which does not take that into account when
building its table of commands and options.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_capabilities.c  | 4 
 src/qemu/qemu_capabilities.h  | 3 +++
 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 +
 3 files changed, 8 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c690cb349..2486d2015 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -439,6 +439,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "virtio-net.tx_queue_size",
   "chardev-reconnect",
   "virtio-gpu.max_outputs",
+
+  /* 270 */
+  "vxhs",
 );
 
 
@@ -1810,6 +1813,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsIntelIOMMU[] = {
 static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/options/+gluster/debug-level", 
QEMU_CAPS_GLUSTER_DEBUG_LEVEL},
 { "blockdev-add/arg-type/+gluster/debug", QEMU_CAPS_GLUSTER_DEBUG_LEVEL},
+{ "blockdev-add/arg-type/+vxhs", QEMU_CAPS_VXHS},
 };
 
 struct virQEMUCapsObjectTypeProps {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 85c390abf..2a0e9c743 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -426,6 +426,9 @@ typedef enum {
 QEMU_CAPS_CHARDEV_RECONNECT, /* -chardev reconnect */
 QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, /* -device 
virtio-(vga|gpu-*),max-outputs= */
 
+/* 270 */
+QEMU_CAPS_VXHS, /* -drive file.driver=vxhs via query-qmp-schema */
+
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
index 604921122..8a31431c0 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
@@ -222,6 +222,7 @@
   
   
   
+  
   201
   0
(v2.10.0)
-- 
2.13.5

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

Re: [libvirt] [PATCH 0/3] qemu: hotplig: Improve error reporting

[Ján Tomko]

On Tue, Sep 12, 2017 at 12:09:43PM +0200, Peter Krempa wrote:

Add helpers to remember existing errors when they might be overwritten
and use it to fix "unknown error" reported when attaching disk with lock
manager enabled.

Peter Krempa (3):
 util: error: Add helpers for saving and restoring of last error
 qemu: hotplug: Use new helpers for storing libvirt errors
 qemu: Restore errors when rolling back disk image state

src/libvirt_private.syms |  2 +
src/qemu/qemu_hotplug.c  | 97 
src/util/virerror.c  | 45 ++
src/util/virerror.h  |  3 ++
4 files changed, 82 insertions(+), 65 deletions(-)


ACK serues

Jan


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

[libvirt] [PATCH v8 09/11] qemu: Introduce qemuDomainPrepareDiskSource

[John Ferlan]

Introduce a function to setup any TLS needs for a disk source.

If there's a configuration or other error setting up the disk source
for TLS, then cause the domain startup to fail.

For VxHS, follow the chardevTLS model where if the src->haveTLS hasn't
been configured, then take the system/global cfg->haveTLS setting for
the storage source *and* mark that we've done so via the tlsFromConfig
setting in storage source.

Next, if we are using TLS, then generate an alias into a virStorageSource
'tlsAlias' field that will be used to create the TLS object and added to
the disk object in order to link the two together for QEMU.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_domain.c| 71 +++
 src/qemu/qemu_domain.h| 11 
 src/qemu/qemu_process.c   |  4 +++
 src/util/virstoragefile.c |  5 +++-
 src/util/virstoragefile.h |  6 
 5 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 05f8e9488..b93b7de63 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7550,6 +7550,77 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def,
 }
 
 
+/* qemuProcessPrepareDiskSourceTLS:
+ * @source: pointer to host interface data for disk device
+ * @diskAlias: alias use for the disk device
+ * @cfg: driver configuration
+ *
+ * Updates host interface TLS encryption setting based on qemu.conf
+ * for disk devices.  This will be presented as "tls='yes|no'" in
+ * live XML of a guest.
+ *
+ * Returns 0 on success, -1 on bad config/failure
+ */
+int
+qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src,
+   const char *diskAlias,
+   virQEMUDriverConfigPtr cfg)
+{
+
+/* VxHS doesn't utilize a password protected server certificate,
+ * so no need to add a secinfo for a secret UUID. */
+if (src->type == VIR_STORAGE_TYPE_NETWORK &&
+src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) {
+
+if (src->haveTLS == VIR_TRISTATE_BOOL_ABSENT) {
+if (cfg->vxhsTLS)
+src->haveTLS = VIR_TRISTATE_BOOL_YES;
+else
+src->haveTLS = VIR_TRISTATE_BOOL_NO;
+src->tlsFromConfig = true;
+}
+
+if (src->haveTLS == VIR_TRISTATE_BOOL_YES) {
+if (!diskAlias) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("disk does not have an alias"));
+return -1;
+}
+
+if (!(src->tlsAlias = qemuAliasTLSObjFromSrcAlias(diskAlias)))
+return -1;
+}
+}
+
+return 0;
+}
+
+
+/* qemuProcessPrepareDiskSource:
+ * @def: live domain definition
+ * @driver: qemu driver
+ *
+ * Iterate through all disk devices to setup/check any that would be
+ * using TLS.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainPrepareDiskSource(virDomainDefPtr def,
+virQEMUDriverConfigPtr cfg)
+{
+size_t i;
+
+for (i = 0; i < def->ndisks; i++) {
+if (qemuDomainPrepareDiskSourceTLS(def->disks[i]->src,
+   def->disks[i]->info.alias,
+   cfg) < 0)
+return -1;
+}
+
+return 0;
+}
+
 
 int
 qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index b291dc308..93db23c2b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -864,6 +864,17 @@ void qemuDomainPrepareChardevSource(virDomainDefPtr def,
 virQEMUDriverConfigPtr cfg)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
+int
+qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src,
+   const char *diskAlias,
+   virQEMUDriverConfigPtr cfg)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
+
+int
+qemuDomainPrepareDiskSource(virDomainDefPtr def,
+virQEMUDriverConfigPtr cfg)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
 int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem)
 ATTRIBUTE_NONNULL(1);
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 099a770e9..f0691ece6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5351,6 +5351,10 @@ qemuProcessPrepareDomain(virConnectPtr conn,
 if (qemuDomainMasterKeyCreate(vm) < 0)
 goto cleanup;
 
+VIR_DEBUG("Prepare disk source backends for TLS");
+if (qemuDomainPrepareDiskSource(vm->def, cfg) < 0)
+goto cleanup;
+
 VIR_DEBUG("Prepare chardev source backends for TLS");
 qemuDomainPrepareChardevSource(vm->def, cfg);
 
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 35f468e35..9cd648d36 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2054,7 +2054,8 @@ virStorageSourceCopy(const 

[libvirt] [PATCH v8 04/11] util: storage: Add JSON backing volume parse for VxHS

[John Ferlan]

From: Ashish Mittal 

Add the backing parse and a test case to verify parsing of VxHS
backing storage.

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 src/util/virstoragefile.c | 37 +
 tests/virstoragetest.c| 11 +++
 2 files changed, 48 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ca306c27b..ba2045369 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3212,6 +3212,40 @@ virStorageSourceParseBackingJSONRaw(virStorageSourcePtr 
src,
 return virStorageSourceParseBackingJSONInternal(src, json);
 }
 
+
+static int
+virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,
+ virJSONValuePtr json,
+ int opaque ATTRIBUTE_UNUSED)
+{
+const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id");
+virJSONValuePtr server = virJSONValueObjectGetObject(json, "server");
+
+if (!vdisk_id || !server) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing 'vdisk-id' or 'server' attribute in "
+ "JSON backing definition for VxHS volume"));
+return -1;
+}
+
+src->type = VIR_STORAGE_TYPE_NETWORK;
+src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS;
+
+if (VIR_STRDUP(src->path, vdisk_id) < 0)
+return -1;
+
+if (VIR_ALLOC_N(src->hosts, 1) < 0)
+return -1;
+src->nhosts = 1;
+
+if (virStorageSourceParseBackingJSONInetSocketAddress(src->hosts,
+  server) < 0)
+return -1;
+
+return 0;
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -3234,6 +3268,7 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"ssh", virStorageSourceParseBackingJSONSSH, 0},
 {"rbd", virStorageSourceParseBackingJSONRBD, 0},
 {"raw", virStorageSourceParseBackingJSONRaw, 0},
+{"vxhs", virStorageSourceParseBackingJSONVxHS, 0},
 };
 
 
@@ -3995,6 +4030,8 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol 
protocol)
 return 0;
 
 case VIR_STORAGE_NET_PROTOCOL_VXHS:
+return ;
+
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 return 0;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 60e3164b0..ffebd4dc1 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1592,6 +1592,17 @@ mymain(void)
"\n"
"  \n"
"\n");
+TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"vxhs\","
+   
"\"vdisk-id\":\"c6718f6b-0401-441d-a8c3-1f0064d75ee0\","
+   "\"server\": { \"type\":\"tcp\","
+  
"\"host\":\"example.com\","
+  "\"port\":\"\""
+   "}"
+  "}"
+"}",
+   "\n"
+   "  \n"
+   "\n");
 #endif /* WITH_YAJL */
 
  cleanup:
-- 
2.13.5

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

Re: [libvirt] [Qemu-devel] QEMU -M nvdimm=on and hotplug

[Igor Mammedov]

On Thu, 14 Sep 2017 13:51:45 +0200
Michal Privoznik  wrote:

> On 09/14/2017 02:33 AM, Haozhong Zhang wrote:
> > On 09/13/17 17:28 +0200, Michal Privoznik wrote:  
> >>
> >> BTW: I ran a migration from no nvdimm qemu to one that had -M nvdimm=on
> >> and guest migrated happily. So looks like guest ABI is stable (or at
> >> least stable enough not to crash). But since ACPI table is changed I
> >> doubt that.  
> > 
> > One example that may cause trouble is that
> > 1/ Guest OS got a pointer to an ACPI table A on the source host (w/o 
> > nvdimm=on)
> > 2/ After migrating to the destination host (w/ nvdimm=on), the
> >location of ACPI table A is occupied by NFIT. If guest OS tries to
> >access ACPI table A via the pointer in step 1/, then it will access
> >the wrong table.
> >   
> 
> Ah, you're right. So it a guest ABI breakage to add nvdimm=on. IOW,
> libvirt can't safely add that onto command line. Well we could for
> freshly started guest and not those which are just being migrated. But
> that increases attack surface so it's not safe either. Okay, I'll stick
> with the latest proposal (expose this in domain XML and let users turn
> it on if they want to).
also note that it depends on memory hotplug being enabled '-m xxx,slots+maxmem'


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

[libvirt] [PATCH v8 11/11] qemu: Add TLS support for Veritas HyperScale (VxHS)

[John Ferlan]

From: Ashish Mittal 

Alter qemu command line generation in order to possibly add TLS for
a suitably configured domain.

Sample TLS args generated by libvirt -

-object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
endpoint=client,verify-peer=yes \
-drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
file.server.type=tcp,file.server.host=192.168.0.1,\
file.server.port=,format=raw,if=none,\
id=drive-virtio-disk0,cache=none \
-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
id=virtio-disk0

Update the qemuxml2argvtest with a couple of examples. One for a
simple case and the other a bit more complex where multiple VxHS disks
are added where at least one uses a VxHS that doesn't require TLS
credentials and thus sets the domain disk source attribute "tls = 'no'".

Update the hotplug to be able to handle processing the tlsAlias whether
it's to add the TLS object when hotplugging a disk or to remove the TLS
object when hot unplugging a disk.  The hot plug/unplug code is largely
generic, but the addition code does make the VXHS specific checks only
because it needs to grab the correct config directory and generate the
object as the command line would do.

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 src/qemu/qemu_block.c  |  8 +++
 src/qemu/qemu_command.c| 29 +
 src/qemu/qemu_hotplug.c| 73 ++
 ...-disk-drive-network-tlsx509-multidisk-vxhs.args | 43 +
 ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 50 +++
 ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 30 +
 tests/qemuxml2argvtest.c   |  7 +++
 7 files changed, 240 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index ca6e213b4..458b90d8e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -529,16 +529,24 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr 
src)
 return NULL;
 }
 
+if (src->haveTLS == VIR_TRISTATE_BOOL_YES && !src->tlsAlias) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("VxHS disk does not have TLS alias set"));
+return NULL;
+}
+
 if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, 
true)))
 return NULL;
 
 /* VxHS disk specification example:
  * { driver:"vxhs",
+ *   [tls-creds:"objvirtio-disk0_tls0",]
  *   vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
  *   server:[{type:"tcp", host:"1.2.3.4", port:}]}
  */
 if (virJSONValueObjectCreate(,
  "s:driver", protocol,
+ "S:tls-creds", src->tlsAlias,
  "s:vdisk-id", src->path,
  "a:server", server, NULL) < 0)
 virJSONValueFree(server);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0a3278510..7b98e1947 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -794,6 +794,32 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 }
 
 
+/* qemuBuildDiskTLSx509CommandLine:
+ *
+ * Add TLS object if the disk uses a secure communication channel
+ *
+ * Returns 0 on success, -1 w/ error on some sort of failure.
+ */
+static int
+qemuBuildDiskTLSx509CommandLine(virCommandPtr cmd,
+virQEMUDriverConfigPtr cfg,
+virDomainDiskDefPtr disk,
+virQEMUCapsPtr qemuCaps)
+{
+virStorageSourcePtr src = disk->src;
+
+/* other protocols may be added later */
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
+disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
+return qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
+  false, true, false,
+  disk->info.alias, qemuCaps);
+}
+
+return 0;
+}
+
+
 static char *
 qemuBuildNetworkDriveURI(virStorageSourcePtr src,
  qemuDomainSecretInfoPtr secinfo)
@@ -2221,6 +2247,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
 if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
 return -1;
 
+if (qemuBuildDiskTLSx509CommandLine(cmd, cfg, disk, qemuCaps) < 0)
+return -1;
+
 virCommandAddArg(cmd, "-drive");
 
 if (!(optstr = qemuBuildDriveStr(disk, cfg, driveBoot, 

[libvirt] [PATCH v8 10/11] util: Add virstoragetest to parse/format a tls='yes'

[John Ferlan]

From: Ashish Mittal 

Add a test case to verify TLS arguments are parsed correctly for
a VxHS disk. This includes saving off a found tls-creds into the
storage source @tlsAlias field since that's what's used to link
the TLS object for the authentication credentials and the disk.

Test case verifies that XML is generated correctly for a VxHS disk
having TLS enabled.

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 src/util/virstoragefile.c |  9 +
 tests/virstoragetest.c| 12 
 2 files changed, 21 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 9cd648d36..1fcd7a028 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3225,6 +3225,7 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr 
src,
 {
 const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id");
 virJSONValuePtr server = virJSONValueObjectGetObject(json, "server");
+const char *haveTLS = virJSONValueObjectGetString(json, "tls-creds");
 
 if (!vdisk_id || !server) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -3243,6 +3244,14 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr 
src,
 return -1;
 src->nhosts = 1;
 
+if (haveTLS) {
+VIR_FREE(src->tlsAlias);
+if (VIR_STRDUP(src->tlsAlias, haveTLS) < 0)
+return -1;
+
+src->haveTLS = VIR_TRISTATE_BOOL_YES;
+}
+
 if (virStorageSourceParseBackingJSONInetSocketAddress(src->hosts,
   server) < 0)
 return -1;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index ffebd4dc1..75ad6330b 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1603,6 +1603,18 @@ mymain(void)
"\n"
"  \n"
"\n");
+TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"vxhs\","
+   
"\"vdisk-id\":\"c6718f6b-0401-441d-a8c3-1f0064d75ee0\","
+   "\"server\": { \"type\":\"tcp\","
+  
"\"host\":\"example.com\","
+  "\"port\":\"\""
+   "},"
+   "\"tls-creds\":\"objvirtio-disk0_tls0\""
+  "}"
+"}",
+   "\n"
+   "  \n"
+   "\n");
 #endif /* WITH_YAJL */
 
  cleanup:
-- 
2.13.5

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

[libvirt] [PATCH v8 08/11] util: Add TLS attributes to virStorageSource

[John Ferlan]

From: Ashish Mittal 

Add an optional virTristateBool haveTLS to virStorageSource to
manage whether a storage source will be using TLS.

Sample XML for a VxHS disk:


  
  

  
  


Additionally add a tlsFromConfig boolean to control whether the TLS
setting was due to domain configuration or qemu.conf global setting
in order to decide whether to Format the haveTLS setting for either
a live or saved domain configuration file.

Update the qemuxml2xmltest in order to add a test to show the proper
parsing.

Also update the docs to describe the tls attribute plus clean up the
description in the surrounding area to make the information a bit more
readable rather than one winding paragraph.

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 docs/formatdomain.html.in  | 40 --
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/domain_conf.c | 28 +--
 src/util/virstoragefile.c  |  2 ++
 src/util/virstoragefile.h  |  7 
 ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +
 ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++
 tests/qemuxml2xmltest.c|  1 +
 8 files changed, 137 insertions(+), 12 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4464c..26c00674a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2520,19 +2520,39 @@
   
   The protocol attribute specifies the protocol to
   access to the requested image. Possible values are "nbd",
-  "iscsi", "rbd", "sheepdog", "gluster" or "vxhs".  If the
-  protocol attribute is "rbd", "sheepdog", "gluster"
-  or "vxhs", an additional attribute name is
-  mandatory to specify which volume/image will be used. For "nbd",
-  the name attribute is optional. For "iscsi"
-  (since 1.0.4), the name
-  attribute may include a logical unit number, separated from the
-  target's name by a slash (e.g.,
+  "iscsi", "rbd", "sheepdog", "gluster" or "vxhs".
+
+  If the protocol attribute is "rbd", "sheepdog",
+  "gluster", or "vxhs", an additional attribute name
+  is mandatory to specify which volume/image will be used.
+  
+
+  For "nbd", the name attribute is optional.
+  
+
+  For "iscsi" (since 1.0.4), the
+  name attribute may include a logical unit number,
+  separated from the target's name by a slash (e.g.,
   iqn.2013-07.com.example:iscsi-pool/1). If not
   specified, the default LUN is zero.
-  For "vxhs" (since 3.8.0), the
+  
+
+  For "vxhs" (since 3.8.0), the
   name is the UUID of the volume, assigned by the
-  HyperScale server.
+  HyperScale server. Additionally, an optional attribute
+  tls (QEMU only) can be used to control whether a
+  VxHS block device would utilize a hypervisor configured TLS
+  X.509 certificate environment in order to encrypt the data
+  channel. For the QEMU hypervisor, usage of a TLS environment can
+  also be globally controlled on the host by the
+  vxhs_tls and vxhs_tls_x509_cert_dir or
+  default_tls_x509_cert_dir settings in the file
+  /etc/libvirt/qemu.conf. If vxhs_tls is enabled,
+  then unless the domain tls attribute is set to "no",
+  libvirt will use the host configured TLS environment. If the
+  tls attribute is set to "yes", then regardless of
+  the qemu.conf setting, TLS authentication will be attempted.
+  
   Since 0.8.7
   
 volume
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 76852abb3..bac371ea3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1644,6 +1644,11 @@
 
   
   
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a43b25c31..3684454e8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8114,6 +8114,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
 int ret = -1;
 char *protocol = NULL;
 xmlNodePtr saveNode = ctxt->node;
+char *haveTLS = NULL;
 
 ctxt->node = node;
 
@@ -8147,6 +8148,19 @@ 

[libvirt] [PATCH v8 07/11] conf: Introduce TLS options for VxHS block device clients

[John Ferlan]

From: Ashish Mittal 

Add a new TLS X.509 certificate type - "vxhs". This will handle the
creation of a TLS certificate capability for properly configured
VxHS network block device clients.

The following describes the behavior of TLS for VxHS block device:

  (1) Two new options have been added in /etc/libvirt/qemu.conf
  to control TLS behavior with VxHS block devices
  "vxhs_tls" and "vxhs_tls_x509_cert_dir".
  (2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable
  TLS for VxHS block devices.
  (3) "vxhs_tls_x509_cert_dir" can be set to the full path where the
  TLS CA certificate and the client certificate and keys are saved.
  If this value is missing, the "default_tls_x509_cert_dir" will be
  used instead. If the environment is not configured properly the
  authentication to the VxHS server will fail.

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 src/qemu/libvirtd_qemu.aug |  4 
 src/qemu/qemu.conf | 34 ++
 src/qemu/qemu_conf.c   | 16 
 src/qemu/qemu_conf.h   |  3 +++
 src/qemu/test_libvirtd_qemu.aug.in |  2 ++
 5 files changed, 59 insertions(+)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index e1983d1fd..c19bf3a43 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -115,6 +115,9 @@ module Libvirtd_qemu =
 
let memory_entry = str_entry "memory_backing_dir"
 
+   let vxhs_entry = bool_entry "vxhs_tls"
+ | str_entry "vxhs_tls_x509_cert_dir"
+
(* Each entry in the config is one of the following ... *)
let entry = default_tls_entry
  | vnc_entry
@@ -133,6 +136,7 @@ module Libvirtd_qemu =
  | nvram_entry
  | gluster_debug_level_entry
  | memory_entry
+ | vxhs_entry
 
let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ 
\t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f977e3b71..2d20d790b 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -258,6 +258,40 @@
 #chardev_tls_x509_secret_uuid = "----"
 
 
+# Enable use of TLS encryption for all VxHS network block devices that
+# don't specifically disable.
+#
+# When the VxHS network block device server is set up appropriately,
+# x509 certificates are required for authentication between the clients
+# (qemu processes) and the remote VxHS server.
+#
+# It is necessary to setup CA and issue the client certificate before
+# enabling this.
+#
+#vxhs_tls = 1
+
+
+# In order to override the default TLS certificate location for VxHS
+# device TCP certificates, supply a valid path to the certificate directory.
+# This is used to authenticate the VxHS block device clients to the VxHS
+# server.
+#
+# If the provided path does not exist then the default_tls_x509_cert_dir
+# path will be used.
+#
+# VxHS block device clients expect the client certificate and key to be
+# present in the certificate directory along with the CA master certificate.
+# If using the default environment, default_tls_x509_verify must be configured.
+# The server key as well as secret UUID that would decrypt it is not used.
+# Thus a VxHS directory must contain the following:
+#
+#  ca-cert.pem - the CA master certificate
+#  client-cert.pem - the client certificate signed with the ca-cert.pem
+#  client-key.pem - the client private key
+#
+#vxhs_tls_x509_cert_dir = "/etc/pki/libvirt-vxhs"
+
+
 # In order to override the default TLS certificate location for migration
 # certificates, supply a valid path to the certificate directory. If the
 # provided path does not exist then the default_tls_x509_cert_dir path
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ab5f7cc59..bcf798d08 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -283,6 +283,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 SET_TLS_X509_CERT_DEFAULT(spice);
 SET_TLS_X509_CERT_DEFAULT(chardev);
 SET_TLS_X509_CERT_DEFAULT(migrate);
+SET_TLS_X509_CERT_DEFAULT(vxhs);
 
 #undef SET_TLS_X509_CERT_DEFAULT
 
@@ -380,6 +381,8 @@ static void virQEMUDriverConfigDispose(void *obj)
 VIR_FREE(cfg->chardevTLSx509certdir);
 VIR_FREE(cfg->chardevTLSx509secretUUID);
 
+VIR_FREE(cfg->vxhsTLSx509certdir);
+
 VIR_FREE(cfg->migrateTLSx509certdir);
 VIR_FREE(cfg->migrateTLSx509secretUUID);
 
@@ -457,6 +460,7 @@ 
virQEMUDriverConfigTLSDirResetDefaults(virQEMUDriverConfigPtr cfg)
 CHECK_RESET_CERT_DIR_DEFAULT(spice);
 CHECK_RESET_CERT_DIR_DEFAULT(chardev);
 CHECK_RESET_CERT_DIR_DEFAULT(migrate);
+CHECK_RESET_CERT_DIR_DEFAULT(vxhs);
 
 return 0;
 }
@@ -556,6 +560,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 goto cleanup;
  

[libvirt] [PATCH v8 00/11] Add support for Veritas HyperScale (VxHS) block device protocol

[John Ferlan]

v7: https://www.redhat.com/archives/libvir-list/2017-September/msg00035.html

Patches 1-4 are already ACK'd, but are presented again for completeness

Differences:

 * Former patch1 already pushed (QEMU 2.10 replies and xml changes)

 * Patch 5 is new - to split up the server args into a single server
   JSON object creation helper so that it can be used by VxHS code in
   patch 6

 * Patch 6 uses the new function and alters the .args output to remove
   the ".0"

 * Patches 7-10, no change

 * Patch 11 - adjust the .args output to remove the ".0"

Ashish Mittal (9):
  storage: Introduce VIR_STORAGE_NET_PROTOCOL_VXHS
  docs: Add schema and docs for Veritas HyperScale (VxHS)
  util: storage: Add JSON backing volume parse for VxHS
  qemu: Refactor qemuBlockStorageSourceBuildHostsJSONSocketAddress
  qemu: Add qemu command line generation for a VxHS block device
  conf: Introduce TLS options for VxHS block device clients
  util: Add TLS attributes to virStorageSource
  util: Add virstoragetest to parse/format a tls='yes'
  qemu: Add TLS support for Veritas HyperScale (VxHS)

John Ferlan (2):
  qemu: Detect support for vxhs
  qemu: Introduce qemuDomainPrepareDiskSource

 docs/formatdomain.html.in  |  45 +--
 docs/schemas/domaincommon.rng  |  18 +++
 src/conf/domain_conf.c |  28 +++-
 src/libxl/libxl_conf.c |   1 +
 src/qemu/libvirtd_qemu.aug |   4 +
 src/qemu/qemu.conf |  34 +
 src/qemu/qemu_block.c  | 150 -
 src/qemu/qemu_capabilities.c   |   4 +
 src/qemu/qemu_capabilities.h   |   3 +
 src/qemu/qemu_command.c|  38 ++
 src/qemu/qemu_conf.c   |  16 +++
 src/qemu/qemu_conf.h   |   3 +
 src/qemu/qemu_domain.c |  71 ++
 src/qemu/qemu_domain.h |  11 ++
 src/qemu/qemu_driver.c |   3 +
 src/qemu/qemu_hotplug.c|  73 ++
 src/qemu/qemu_parse_command.c  |  15 +++
 src/qemu/qemu_process.c|  33 +
 src/qemu/test_libvirtd_qemu.aug.in |   2 +
 src/util/virstoragefile.c  |  58 +++-
 src/util/virstoragefile.h  |  14 ++
 src/xenconfig/xen_xl.c |   1 +
 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  |   1 +
 ...-disk-drive-network-tlsx509-multidisk-vxhs.args |  43 ++
 ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml |  50 +++
 ...muxml2argv-disk-drive-network-tlsx509-vxhs.args |  30 +
 ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml |  32 +
 .../qemuxml2argv-disk-drive-network-vxhs.args  |  27 
 .../qemuxml2argv-disk-drive-network-vxhs.xml   |  32 +
 tests/qemuxml2argvtest.c   |   8 ++
 ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml |  34 +
 .../qemuxml2xmlout-disk-drive-network-vxhs.xml |  34 +
 tests/qemuxml2xmltest.c|   2 +
 tests/virstoragetest.c |  23 
 34 files changed, 893 insertions(+), 48 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-vxhs.xml

-- 
2.13.5

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

[libvirt] [PATCH v8 06/11] qemu: Add qemu command line generation for a VxHS block device

[John Ferlan]

From: Ashish Mittal 

The VxHS block device will only use the newer formatting options and
avoid the legacy URI syntax.

An excerpt for a sample QEMU command line is:

  -drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
   file.server.type=tcp,file.server.host=192.168.0.1,\
   file.server.port=,format=raw,if=none,id=drive-virtio-disk0,cache=none \
  -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
   id=virtio-disk0

Update qemuxml2argvtest with a simple test.

Signed-off-by: Ashish Mittal 
Signed-off-by: John Ferlan 
---
 src/qemu/qemu_block.c  | 37 +-
 src/qemu/qemu_command.c| 10 +-
 src/qemu/qemu_parse_command.c  | 16 +-
 src/qemu/qemu_process.c| 29 +
 .../qemuxml2argv-disk-drive-network-vxhs.args  | 27 
 tests/qemuxml2argvtest.c   |  1 +
 6 files changed, 117 insertions(+), 3 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index c97b787c5..ca6e213b4 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -516,6 +516,37 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr 
src)
 }
 
 
+static virJSONValuePtr
+qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
+{
+const char *protocol = virStorageNetProtocolTypeToString(src->protocol);
+virJSONValuePtr server = NULL;
+virJSONValuePtr ret = NULL;
+
+if (src->nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("VxHS protocol accepts only one host"));
+return NULL;
+}
+
+if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, 
true)))
+return NULL;
+
+/* VxHS disk specification example:
+ * { driver:"vxhs",
+ *   vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
+ *   server:[{type:"tcp", host:"1.2.3.4", port:}]}
+ */
+if (virJSONValueObjectCreate(,
+ "s:driver", protocol,
+ "s:vdisk-id", src->path,
+ "a:server", server, NULL) < 0)
+virJSONValueFree(server);
+
+return ret;
+}
+
+
 /**
  * qemuBlockStorageSourceGetBackendProps:
  * @src: disk source
@@ -546,6 +577,11 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr 
src)
 goto cleanup;
 break;
 
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
+if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src)))
+goto cleanup;
+break;
+
 case VIR_STORAGE_NET_PROTOCOL_NBD:
 case VIR_STORAGE_NET_PROTOCOL_RBD:
 case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
@@ -556,7 +592,6 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr 
src)
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
-case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 break;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 720530daa..0a3278510 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -994,12 +994,16 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
 ret = virBufferContentAndReset();
 break;
 
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("VxHS protocol does not support URI syntax"));
+goto cleanup;
+
 case VIR_STORAGE_NET_PROTOCOL_SSH:
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'ssh' protocol is not yet supported"));
 goto cleanup;
 
-case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1329,6 +1333,10 @@ qemuDiskSourceNeedsProps(virStorageSourcePtr src)
 src->nhosts > 1)
 return true;
 
+if (actualType == VIR_STORAGE_TYPE_NETWORK &&
+src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS)
+return true;
+
 return false;
 }
 
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 9190a37ba..6286c2e7a 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -736,6 +736,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
 if (VIR_STRDUP(def->src->path, vdi) < 0)
 goto error;
 }
+} else if (STRPREFIX(def->src->path, "vxhs:")) {
+

[libvirt] [PATCH python] Skip sparseRecvAll / sparseSendAll in sanity test

[Daniel P. Berrange]

The sanity test check aims to ensure that every function listed in
the Python code maps to a corresponding C function. The Sparse
send/recv methods are special though - we're never calling the
corresponding C APIs, instead we have a pure python impl.

Signed-off-by: Daniel P. Berrange 
---
 sanitytest.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sanitytest.py b/sanitytest.py
index deec200..a5cb01b 100644
--- a/sanitytest.py
+++ b/sanitytest.py
@@ -351,7 +351,8 @@ for klass in gotfunctions:
 for func in sorted(gotfunctions[klass]):
 # These are pure python methods with no C APi
 if func in ["connect", "getConnect", "domain", "getDomain",
-"virEventInvokeFreeCallback"]:
+"virEventInvokeFreeCallback",
+"sparseRecvAll", "sparseSendAll"]:
 continue
 
 key = "%s.%s" % (klass, func)
-- 
2.13.5

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

Re: [libvirt] [libvirt-python][PATCH] maint: Require libvirt-3.4.0 at least

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 01:41:08PM +0200, Michal Privoznik wrote:
> Currently, we require 0.9.11. However, some APIs are missing
> there and thus sanity check fails:
> 
> DEBUG: /usr/bin/python sanitytest.py build/lib.linux-s390x-2.7 
> /usr/share/libvirt/api/libvirt-api.xml
> DEBUG: FAIL virStream.sparseRecvAll   (Python API not mapped to C)
> DEBUG: FAIL virStream.sparseSendAll   (Python API not mapped to C)
> DEBUG: error: command '/usr/bin/python' failed with exit status 1
> 
> I'm not sure how to fix that so raising minimal required libvirt
> version is the solution.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  setup.py   |   2 +-
>  README |   2 +-
>  libvirt-override.c | 149 
> -
>  3 files changed, 2 insertions(+), 151 deletions(-)
> 
> diff --git a/setup.py b/setup.py
> index f33ff1a..f929eb2 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -17,7 +17,7 @@ import re
>  import shutil
>  import time
>  
> -MIN_LIBVIRT = "0.9.11"
> +MIN_LIBVIRT = "3.4.0"

NACK, we cannot do this - it will break many people and apps (OpenStack
in particular) who expect latest libvirt on pypi to work with historical
C libs.


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] QEMU -M nvdimm=on and hotplug

[Michal Privoznik]

On 09/14/2017 02:33 AM, Haozhong Zhang wrote:
> On 09/13/17 17:28 +0200, Michal Privoznik wrote:
>>
>> BTW: I ran a migration from no nvdimm qemu to one that had -M nvdimm=on
>> and guest migrated happily. So looks like guest ABI is stable (or at
>> least stable enough not to crash). But since ACPI table is changed I
>> doubt that.
> 
> One example that may cause trouble is that
> 1/ Guest OS got a pointer to an ACPI table A on the source host (w/o 
> nvdimm=on)
> 2/ After migrating to the destination host (w/ nvdimm=on), the
>location of ACPI table A is occupied by NFIT. If guest OS tries to
>access ACPI table A via the pointer in step 1/, then it will access
>the wrong table.
> 

Ah, you're right. So it a guest ABI breakage to add nvdimm=on. IOW,
libvirt can't safely add that onto command line. Well we could for
freshly started guest and not those which are just being migrated. But
that increases attack surface so it's not safe either. Okay, I'll stick
with the latest proposal (expose this in domain XML and let users turn
it on if they want to).

Thanks.

Michal

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

[libvirt] [libvirt-python][PATCH] maint: Require libvirt-3.4.0 at least

[Michal Privoznik]

Currently, we require 0.9.11. However, some APIs are missing
there and thus sanity check fails:

DEBUG: /usr/bin/python sanitytest.py build/lib.linux-s390x-2.7 
/usr/share/libvirt/api/libvirt-api.xml
DEBUG: FAIL virStream.sparseRecvAll   (Python API not mapped to C)
DEBUG: FAIL virStream.sparseSendAll   (Python API not mapped to C)
DEBUG: error: command '/usr/bin/python' failed with exit status 1

I'm not sure how to fix that so raising minimal required libvirt
version is the solution.

Signed-off-by: Michal Privoznik 
---
 setup.py   |   2 +-
 README |   2 +-
 libvirt-override.c | 149 -
 3 files changed, 2 insertions(+), 151 deletions(-)

diff --git a/setup.py b/setup.py
index f33ff1a..f929eb2 100755
--- a/setup.py
+++ b/setup.py
@@ -17,7 +17,7 @@ import re
 import shutil
 import time
 
-MIN_LIBVIRT = "0.9.11"
+MIN_LIBVIRT = "3.4.0"
 MIN_LIBVIRT_LXC = "1.0.2"
 
 # Hack to stop 'pip install' failing with error
diff --git a/README b/README
index 96082f0..bc3ee77 100644
--- a/README
+++ b/README
@@ -5,7 +5,7 @@ This package provides a python binding to the libvirt.so,
 libvirt-qemu.so and libvirt-lxc.so library APIs.
 
 It is written to build against any version of libvirt that
-is 0.9.11 or newer.
+is 3.4.0 or newer.
 
 This code is distributed under the terms of the LGPL version
 2 or later.
diff --git a/libvirt-override.c b/libvirt-override.c
index 9eba4ed..280856d 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -66,20 +66,9 @@ getPyNodeCPUCount(virConnectPtr conn)
 {
 int i_retval;
 
-#if LIBVIR_CHECK_VERSION(1, 0, 0)
 LIBVIRT_BEGIN_ALLOW_THREADS;
 i_retval = virNodeGetCPUMap(conn, NULL, NULL, 0);
 LIBVIRT_END_ALLOW_THREADS;
-#else /* fallback: use nodeinfo */
-virNodeInfo nodeinfo;
-
-LIBVIRT_BEGIN_ALLOW_THREADS;
-i_retval = virNodeGetInfo(conn, );
-LIBVIRT_END_ALLOW_THREADS;
-
-if (i_retval >= 0)
-i_retval = VIR_NODEINFO_MAXCPUS(nodeinfo);
-#endif /* LIBVIR_CHECK_VERSION(1, 0, 0) */
 
 return i_retval;
 }
@@ -398,14 +387,12 @@ libvirt_virDomainMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED,
 case VIR_DOMAIN_MEMORY_STAT_RSS:
 key = libvirt_constcharPtrWrap("rss");
 break;
-#if LIBVIR_CHECK_VERSION(2, 1, 0)
 case VIR_DOMAIN_MEMORY_STAT_USABLE:
 key = libvirt_constcharPtrWrap("usable");
 break;
 case VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE:
 key = libvirt_constcharPtrWrap("last_update");
 break;
-#endif /* LIBVIR_CHECK_VERSION(2, 1, 0) */
 default:
 continue;
 }
@@ -1451,7 +1438,6 @@ libvirt_virDomainGetVcpuPinInfo(PyObject *self 
ATTRIBUTE_UNUSED,
 }
 
 
-#if LIBVIR_CHECK_VERSION(0, 10, 0)
 static PyObject *
 libvirt_virDomainPinEmulator(PyObject *self ATTRIBUTE_UNUSED,
  PyObject *args)
@@ -1547,9 +1533,7 @@ libvirt_virDomainGetEmulatorPinInfo(PyObject *self 
ATTRIBUTE_UNUSED,
 Py_CLEAR(pycpumap);
 goto cleanup;
 }
-#endif /* LIBVIR_CHECK_VERSION(0, 10, 0) */
 
-#if LIBVIR_CHECK_VERSION(1, 2, 14)
 static PyObject *
 libvirt_virDomainGetIOThreadInfo(PyObject *self ATTRIBUTE_UNUSED,
  PyObject *args)
@@ -1668,7 +1652,6 @@ libvirt_virDomainPinIOThread(PyObject *self 
ATTRIBUTE_UNUSED,
 return ret;
 }
 
-#endif /* LIBVIR_CHECK_VERSION(1, 2, 14) */
 
 /
  * *
@@ -2037,7 +2020,6 @@ libvirt_virConnectGetVersion(PyObject *self 
ATTRIBUTE_UNUSED,
 return libvirt_intWrap(hvVersion);
 }
 
-#if LIBVIR_CHECK_VERSION(1, 1, 3)
 static PyObject *
 libvirt_virConnectGetCPUModelNames(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args)
@@ -2083,7 +2065,6 @@ libvirt_virConnectGetCPUModelNames(PyObject *self 
ATTRIBUTE_UNUSED,
 Py_CLEAR(rv);
 goto done;
 }
-#endif /* LIBVIR_CHECK_VERSION(1, 1, 3) */
 
 static PyObject *
 libvirt_virConnectGetLibVersion(PyObject *self ATTRIBUTE_UNUSED,
@@ -2162,7 +2143,6 @@ libvirt_virConnectListDomainsID(PyObject *self 
ATTRIBUTE_UNUSED,
 return NULL;
 }
 
-#if LIBVIR_CHECK_VERSION(0, 9, 13)
 static PyObject *
 libvirt_virConnectListAllDomains(PyObject *self ATTRIBUTE_UNUSED,
  PyObject *args)
@@ -2206,7 +2186,6 @@ libvirt_virConnectListAllDomains(PyObject *self 
ATTRIBUTE_UNUSED,
 Py_CLEAR(py_retval);
 goto cleanup;
 }
-#endif /* LIBVIR_CHECK_VERSION(0, 9, 13) */
 
 static PyObject *
 libvirt_virConnectListDefinedDomains(PyObject *self ATTRIBUTE_UNUSED,
@@ -2317,7 +2296,6 @@ libvirt_virDomainSnapshotListNames(PyObject *self 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
-#if LIBVIR_CHECK_VERSION(0, 9, 13)
 static PyObject *
 libvirt_virDomainListAllSnapshots(PyObject *self ATTRIBUTE_UNUSED,
   

Re: [libvirt] Exposing mem-path in domain XML

[Michal Privoznik]

On 09/06/2017 01:42 PM, Daniel P. Berrange wrote:
> On Wed, Sep 06, 2017 at 01:35:45PM +0200, Michal Privoznik wrote:
>> On 09/05/2017 04:07 PM, Daniel P. Berrange wrote:
>>> On Tue, Sep 05, 2017 at 03:59:09PM +0200, Michal Privoznik wrote:
 On 07/28/2017 10:59 AM, Daniel P. Berrange wrote:
> On Fri, Jul 28, 2017 at 10:45:21AM +0200, Michal Privoznik wrote:
>> On 07/27/2017 03:50 PM, Daniel P. Berrange wrote:
>>> On Thu, Jul 27, 2017 at 02:11:25PM +0200, Michal Privoznik wrote:
 Dear list,

 there is the following bug [1] which I'm not quite sure how to grasp. 
 So
 there is this application/infrastructure called Kove [2] that allows 
 you
 to have memory for your application stored on a distant host in network
 and basically fetch needed region on pagefault. Now imagine that
 somebody wants to use it for backing up domain memory. However, the way
 that the tool works is it has some kernel module and then some userland
 binary that is fed with the path of the mmaped file. I don't know all
 the details, but the point is, in order to let users use this we need 
 to
 expose the paths for mem-path for the guest memory. I know we did not
 want to do this in the past, but now it looks like we don't have a way
 around it, do we?
>>>
>>> We don't want to expose the concept of paths in the XML because this is
>>> a linux specific way to configure hugepages / shared memory. So we hide
>>> the particular path used in the internal impl of the QEMU driver, and
>>> or via the qemu.conf global config file. I don't really want to change
>>> that approach, particularly if the only reason is to integrate with a
>>> closed source binary like Kove. 
>>
>> Yep, I agree with that. However, if you read the discussion in the
>> linked bug you'll find that they need to know what file in the
>> memory_backing_dir (from qemu.conf) corresponds to which domain. The
>> reported suggested using UUID based filenames, which I fear is not
>> enough because one can have multiple  -s configured
>> for their domain. But I guess we could go with:
>>
>> ${memory_backing_dir}/${domName}for generic memory
>> ${memory_backing_dir}/${domName}_N  for Nth 
>
> This feels like it is going to lead to hell when you add in memory
> hotplug/unplug, with inevitable races.
>
>> BTW: IIUC they want predictable names because they need to create the
>> files before spawning qemu so that they are picked by qemu instead of
>> using temporary names.
>
> I would like to know why they even need to associate particular memory
> files with particular QEMU processes. eg if they're just exposing a
> new type of tmpfs filesystem from the kernel why does it matter what
> each file is used for.

 This might get you answer:

 https://bugzilla.redhat.com/show_bug.cgi?id=1461214#c4

 So the way I understand it is that they will create the files, and
 provide us with paths. So luckily, we don't have to make up the paths on
 our own.
>>>
>>> IOW it is pretending to be tmpfs except it is not behaving like tmpfs.
>>> This doesn't really make me any more inclined to support this closed
>>> source stuff in libvirt.
>>
>> Yeah, that's my feeling too. So, what about the following: let's assume
>> they will fix their code so that it is proper tmpfs. Libvirt can then
>> behave to it just like it is already doing so for hugetlbfs. For us
>> it'll be just yet another type of hugepages. I mean, for hugepages we
>> already create /hupages/mount/point/libvirt/$domain per each domain so
>> the separation is there (even though this is considered internal impl),
>> since it would be a proper tmpfs they can see the pid of qemu which is
>> trying to mmap() (and take the name or whatever unique ID they want from
>> there).
> 
> Yep, we can at least make a reasonable guarantee that all files belonging
> to a single QEMU process will always be within the same sub-directory.
> This allows the kmod to distinguish 2 files owned by separate VMs, from 2
> files owned by the same VM and do what's needed. I don't see why it would
> need to care about naming conventions beyond the layout.
> 
>> I guess what I'm trying to ask is if it was proper tmpfs, we would be
>> okay with it, wouldn't we?
> 
> If it is indistinguishable from tmpfs/hugetlbfs from libvirt's POV, we
> should be fine -  at most you would need /etc/libvirt/qemu.conf change
> to explicitly point at the custom mount point if libvirt doesn't
> auto-detect the right one.
> 

Zack, can you join the discussion and tell us if our design sounds
reasonable to you?

Michal

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

[libvirt] [PATCH libvirt-glib 0/3] Doc updates

[Guido Günther]

Hi,
here are some doc updates I found on a long unused branch but they still apply.
Cheers,
 -- Guido

Guido Günther (3):
  Add some missing docs
  streams: fix references
  gvir_stream_send: make it obvious that we send bytes

 libvirt-gobject/libvirt-gobject-connection.c | 15 ++-
 libvirt-gobject/libvirt-gobject-domain.c |  2 +-
 libvirt-gobject/libvirt-gobject-stream.c | 38 ++--
 3 files changed, 39 insertions(+), 16 deletions(-)

-- 
2.14.1

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

[libvirt] [PATCH libvirt-glib 1/3] Add some missing docs

[Guido Günther]

---
 libvirt-gobject/libvirt-gobject-connection.c | 15 ++-
 libvirt-gobject/libvirt-gobject-domain.c |  2 +-
 libvirt-gobject/libvirt-gobject-stream.c | 10 ++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index 3f17265..47a3f0c 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -487,6 +487,10 @@ static gboolean _gvir_connection_open(GVirConnection *conn,
  * gvir_connection_open:
  * @conn: a #GVirConnection
  * @cancellable: (allow-none)(transfer none): cancellation object
+ *
+ * Open a connection
+ *
+ * Returns: TRUE on success, FALSE on error
  */
 gboolean gvir_connection_open(GVirConnection *conn,
   GCancellable *cancellable,
@@ -495,6 +499,15 @@ gboolean gvir_connection_open(GVirConnection *conn,
 return _gvir_connection_open(conn, FALSE, cancellable, err);
 }
 
+/**
+ * gvir_connection_open_read_only:
+ * @conn: a #GVirConnection
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ *
+ * Open a connection read only
+ *
+ * Returns: TRUE on success, FALSE on error
+ */
 gboolean gvir_connection_open_read_only(GVirConnection *conn,
 GCancellable *cancellable,
 GError **err)
@@ -1340,7 +1353,7 @@ G_DEFINE_BOXED_TYPE(GVirConnectionHandle, 
gvir_connection_handle,
  * @conn: a #GVirConnection
  * @flags: flags to use for the stream
  *
- * Return value: (transfer full): a #GVirStream stream, or NULL.The returned
+ * Return value: (transfer full): a #GVirStream stream, or NULL. The returned
  * object should be unreffed with g_object_unref() when no longer needed.
  */
 GVirStream *gvir_connection_get_stream(GVirConnection *self,
diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 7be936d..3bfc65f 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -1088,8 +1088,8 @@ end:
 /**
  * gvir_domain_open_console:
  * @dom: (transfer none): the domain
- * @devname: (transfer none)(allow-none): the device name
  * @stream: (transfer none): stream to use as output
+ * @devname: (transfer none)(allow-none): the device name
  * @flags: extra flags, currently unused
  *
  * Open a text console for the domain @dom, connecting it to the
diff --git a/libvirt-gobject/libvirt-gobject-stream.c 
b/libvirt-gobject/libvirt-gobject-stream.c
index a518a19..b6bf774 100644
--- a/libvirt-gobject/libvirt-gobject-stream.c
+++ b/libvirt-gobject/libvirt-gobject-stream.c
@@ -67,6 +67,16 @@ enum {
 };
 
 
+/**
+ * SECTION:gvir-stream
+ * @short_description: Streaming operations for domain input and output
+ *
+ * #GVirStream implements #GInputStream and #GOutputStream for reading from and
+ * writing to a domain e.g. via a text based console on a emulated serial port.
+ *
+ * It is usually created via #gvir_connection_get_stream.
+ */
+
 #define GVIR_STREAM_ERROR gvir_stream_error_quark()
 
 static void gvir_stream_update_events(GVirStream *stream);
-- 
2.14.1

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

[libvirt] [PATCH libvirt-glib 3/3] gvir_stream_send: make it obvious that we send bytes

[Guido Günther]

Fix doc and use a proper variable name
---
 libvirt-gobject/libvirt-gobject-stream.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-stream.c 
b/libvirt-gobject/libvirt-gobject-stream.c
index 296c00e..93788b5 100644
--- a/libvirt-gobject/libvirt-gobject-stream.c
+++ b/libvirt-gobject/libvirt-gobject-stream.c
@@ -440,15 +440,15 @@ gvir_stream_receive_all(GVirStream *self,
  * @cancellable: (allow-none): a %GCancellable or %NULL
  * @error: #GError for error reporting, or %NULL to ignore.
  *
- * Send data (up to @size bytes) from a stream.
+ * Send data (up to @size bytes) to a stream.
  * On error -1 is returned and @error is set accordingly.
  *
- * gvir_stream_send() can return any number of bytes, up to
- * @size. If more than @size bytes have been sendd, the additional
- * data will be returned in future calls to gvir_stream_send().
+ * gvir_stream_send() can send any number of bytes, up to
+ * @size. If less than @size bytes have been send, the additional
+ * data can be sent in future calls to gvir_stream_send().
  *
- * If there is no data available, a %G_IO_ERROR_WOULD_BLOCK error will be
- * returned.
+ * If the receiving end would block, a %G_IO_ERROR_WOULD_BLOCK error
+ * will be returned.
  *
  * Returns: Number of bytes written.
  */
@@ -458,7 +458,7 @@ gssize gvir_stream_send(GVirStream *self,
 GCancellable *cancellable,
 GError **error)
 {
-int got;
+int sent;
 
 g_return_val_if_fail(GVIR_IS_STREAM(self), -1);
 g_return_val_if_fail(buffer != NULL, -1);
@@ -468,17 +468,17 @@ gssize gvir_stream_send(GVirStream *self,
 if (g_cancellable_set_error_if_cancelled (cancellable, error))
 return -1;
 
-got = virStreamSend(self->priv->handle, buffer, size);
+sent = virStreamSend(self->priv->handle, buffer, size);
 
-if (got == -2) {  /* blocking */
+if (sent == -2) {  /* blocking */
 g_set_error_literal(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK,
 _("virStreamSend call would block"));
-} else if (got < 0) {
+} else if (sent < 0) {
 g_set_error(error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
-_("Got virStreamRecv error in %s"), G_STRFUNC);
+_("Sent virStreamRecv error in %s"), G_STRFUNC);
 }
 
-return got;
+return sent;
 }
 
 struct stream_source_helper {
-- 
2.14.1

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

[libvirt] [PATCH libvirt-glib 2/3] streams: fix references

[Guido Günther]

---
 libvirt-gobject/libvirt-gobject-stream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-stream.c 
b/libvirt-gobject/libvirt-gobject-stream.c
index b6bf774..296c00e 100644
--- a/libvirt-gobject/libvirt-gobject-stream.c
+++ b/libvirt-gobject/libvirt-gobject-stream.c
@@ -396,7 +396,7 @@ stream_sink(virStreamPtr st G_GNUC_UNUSED,
  *
  * Receive the entire data stream, sending the data to the
  * requested data sink. This is simply a convenient alternative
- * to virStreamRecv, for apps that do blocking-I/o.
+ * to #gvir_vir_stream_receive, for apps that do blocking-I/o.
  *
  * Returns: the number of bytes consumed or -1 upon error
  */
@@ -512,7 +512,7 @@ stream_source(virStreamPtr st G_GNUC_UNUSED,
  *
  * Send the entire data stream, sending the data to the
  * requested data source. This is simply a convenient alternative
- * to virStreamRecv, for apps that do blocking-I/o.
+ * to #gvir_stream_send, for apps that do blocking-I/o.
  *
  * Returns: the number of bytes consumed or -1 upon error
  */
-- 
2.14.1

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

Re: [libvirt] [PATCH v4 1/2] conf, docs: Add qcow2 cache configuration support

[Liu Qing]

On Thu, Sep 14, 2017 at 12:56:30PM +0200, Peter Krempa wrote:
> On Thu, Sep 14, 2017 at 16:37:03 +0800, Liu Qing wrote:
> > On Thu, Sep 14, 2017 at 10:02:40AM +0200, Peter Krempa wrote:
> > > On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:
> 
> [...]
> 
> > > One more thing. The design you've proposed is really not user friendly.
> > > The user has to read a lenghty document, then inquire the parameters for
> > > every single qcow2 image, do the calculations and set it in the XML.
> > > This might be okay for higher level management tools, but not for users
> > > who use libvirt directly.
> > I agree this is not user friendly. I only give the user a choice, maybe
> > not the best, right now. Any suggestion will be greatly appreicated.
> > For our company, this feature will be an add-on to Openstack.
> 
> This means that you will have to deal with everything that I've pointed
> out. How are you planning to expose this to the user? And which
> calculations are you going to use?
We will not expose this to the user on Openstack level.
We have different volume types like performance and capacity. For
performance volumes we will cache as much space as possible. Free memory
left in the host will be counted in. For capacity volume we will set a
max cache size.
The cache setting strategy be implemented in a higher level management
tool seems much resaonable to me. As this will be much more flexible.


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

[libvirt] [PATCH 0/7] qemu: Filter CPU features returned by qemuConnectBaselineCPU

[Jiri Denemark]

The host CPU definitions reported in the capabilities XML may contain
CPU features unknown to QEMU, but the result of virConnectBaselineCPU is
supposed to be directly usable as a guest CPU definition and thus it
should only contain features QEMU knows about.

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

Jiri Denemark (7):
  cpu_conf: Introduce virCPUDefList{Parse,Free}
  cpu: Use virCPUDefListParse in cpuBaselineXML
  cpu: Don't log CPU models in cpuBaselineXML
  cpu: Drop cpuBaselineXML
  qemu: Pass virArch * to virQEMUCapsCPUFilterFeatures
  qemu: Publish virQEMUCapsCPUFilterFeatures
  qemu: Filter CPU features returned by qemuConnectBaselineCPU

 src/bhyve/bhyve_driver.c |  22 +++--
 src/conf/cpu_conf.c  |  78 
 src/conf/cpu_conf.h  |   7 +++
 src/cpu/cpu.c| 104 ---
 src/cpu/cpu.h|   7 ---
 src/libvirt_private.syms |   3 +-
 src/libxl/libxl_driver.c |  22 +++--
 src/qemu/qemu_capabilities.c |   8 ++--
 src/qemu/qemu_capabilities.h |   3 ++
 src/qemu/qemu_driver.c   |  32 +++--
 src/test/test_driver.c   |  22 +++--
 src/vz/vz_driver.c   |  22 -
 12 files changed, 201 insertions(+), 129 deletions(-)

-- 
2.14.1

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

[libvirt] [PATCH 1/7] cpu_conf: Introduce virCPUDefList{Parse, Free}

[Jiri Denemark]

For parsing a list of CPU XMLs into a NULL-terminated list of CPU defs.

Signed-off-by: Jiri Denemark 
---
 src/conf/cpu_conf.c  | 78 
 src/conf/cpu_conf.h  |  7 +
 src/libvirt_private.syms |  2 ++
 3 files changed, 87 insertions(+)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index c21d11d244..7514842059 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -29,9 +29,12 @@
 #include "cpu_conf.h"
 #include "domain_conf.h"
 #include "virstring.h"
+#include "virlog.h"
 
 #define VIR_FROM_THIS VIR_FROM_CPU
 
+VIR_LOG_INIT("conf.cpu_conf");
+
 VIR_ENUM_IMPL(virCPU, VIR_CPU_TYPE_LAST,
   "host", "guest", "auto")
 
@@ -939,3 +942,78 @@ virCPUDefIsEqual(virCPUDefPtr src,
  cleanup:
 return identical;
 }
+
+
+/*
+ * Parses a list of CPU XMLs into a NULL-terminated list of CPU defs.
+ */
+virCPUDefPtr *
+virCPUDefListParse(const char **xmlCPUs,
+   unsigned int ncpus,
+   virCPUType cpuType)
+{
+xmlDocPtr doc = NULL;
+xmlXPathContextPtr ctxt = NULL;
+virCPUDefPtr *cpus = NULL;
+size_t i;
+
+VIR_DEBUG("xmlCPUs=%p, ncpus=%u", xmlCPUs, ncpus);
+
+if (xmlCPUs) {
+for (i = 0; i < ncpus; i++)
+VIR_DEBUG("xmlCPUs[%zu]=%s", i, NULLSTR(xmlCPUs[i]));
+}
+
+if (!xmlCPUs && ncpus != 0) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("nonzero ncpus doesn't match with NULL xmlCPUs"));
+goto error;
+}
+
+if (ncpus < 1) {
+virReportError(VIR_ERR_INVALID_ARG, "%s", _("no CPUs given"));
+goto error;
+}
+
+if (VIR_ALLOC_N(cpus, ncpus + 1))
+goto error;
+
+for (i = 0; i < ncpus; i++) {
+if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], _("(CPU_definition)"), 
)))
+goto error;
+
+if (virCPUDefParseXML(ctxt, NULL, cpuType, [i]) < 0)
+goto error;
+
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(doc);
+ctxt = NULL;
+doc = NULL;
+}
+
+return cpus;
+
+ error:
+virCPUDefListFree(cpus);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(doc);
+return NULL;
+}
+
+
+/*
+ * Frees NULL-terminated list of CPUs created by virCPUDefListParse.
+ */
+void
+virCPUDefListFree(virCPUDefPtr *cpus)
+{
+virCPUDefPtr *cpu;
+
+if (!cpus)
+return;
+
+for (cpu = cpus; *cpu != NULL; cpu++)
+virCPUDefFree(*cpu);
+
+VIR_FREE(cpus);
+}
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index b44974f47e..d3e2c84102 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -218,4 +218,11 @@ virCPUDefUpdateFeature(virCPUDefPtr cpu,
const char *name,
int policy);
 
+virCPUDefPtr *
+virCPUDefListParse(const char **xmlCPUs,
+   unsigned int ncpus,
+   virCPUType cpuType);
+void
+virCPUDefListFree(virCPUDefPtr *cpus);
+
 #endif /* __VIR_CPU_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 247d1175ba..857e417f94 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -82,6 +82,8 @@ virCPUDefFree;
 virCPUDefFreeFeatures;
 virCPUDefFreeModel;
 virCPUDefIsEqual;
+virCPUDefListFree;
+virCPUDefListParse;
 virCPUDefParseXML;
 virCPUDefStealModel;
 virCPUDefUpdateFeature;
-- 
2.14.1

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

[libvirt] [PATCH 2/7] cpu: Use virCPUDefListParse in cpuBaselineXML

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c | 52 ++--
 1 file changed, 6 insertions(+), 46 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 96160901e1..bf3c6d53dd 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -523,11 +523,9 @@ cpuBaselineXML(const char **xmlCPUs,
unsigned int nmodels,
unsigned int flags)
 {
-xmlDocPtr doc = NULL;
-xmlXPathContextPtr ctxt = NULL;
 virCPUDefPtr *cpus = NULL;
 virCPUDefPtr cpu = NULL;
-char *cpustr;
+char *cpustr = NULL;
 size_t i;
 
 VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels);
@@ -535,67 +533,29 @@ cpuBaselineXML(const char **xmlCPUs,
 virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
   VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
 
-if (xmlCPUs) {
-for (i = 0; i < ncpus; i++)
-VIR_DEBUG("xmlCPUs[%zu]=%s", i, NULLSTR(xmlCPUs[i]));
-}
 if (models) {
 for (i = 0; i < nmodels; i++)
 VIR_DEBUG("models[%zu]=%s", i, NULLSTR(models[i]));
 }
 
-if (xmlCPUs == NULL && ncpus != 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("nonzero ncpus doesn't match with NULL 
xmlCPUs"));
-return NULL;
-}
-
-if (ncpus < 1) {
-virReportError(VIR_ERR_INVALID_ARG, "%s", _("No CPUs given"));
-return NULL;
-}
-
-if (VIR_ALLOC_N(cpus, ncpus))
-goto error;
-
-for (i = 0; i < ncpus; i++) {
-if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], _("(CPU_definition)"), 
)))
-goto error;
-
-if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_HOST, [i]) < 0)
-goto error;
-
-xmlXPathFreeContext(ctxt);
-xmlFreeDoc(doc);
-ctxt = NULL;
-doc = NULL;
-}
+if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST)))
+goto cleanup;
 
 if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels,
 !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE
-goto error;
+goto cleanup;
 
 if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
 virCPUExpandFeatures(cpus[0]->arch, cpu) < 0)
-goto error;
+goto cleanup;
 
 cpustr = virCPUDefFormat(cpu, NULL, false);
 
  cleanup:
-if (cpus) {
-for (i = 0; i < ncpus; i++)
-virCPUDefFree(cpus[i]);
-VIR_FREE(cpus);
-}
+virCPUDefListFree(cpus);
 virCPUDefFree(cpu);
-xmlXPathFreeContext(ctxt);
-xmlFreeDoc(doc);
 
 return cpustr;
-
- error:
-cpustr = NULL;
-goto cleanup;
 }
 
 
-- 
2.14.1

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

Re: [libvirt] [PATCH v7 06.5/11] merge with previous...

[John Ferlan]

On 09/14/2017 04:13 AM, Peter Krempa wrote:
> On Tue, Sep 12, 2017 at 14:39:58 -0400, John Ferlan wrote:
>> This patch would be merged with the existing 06/11 patch for the
>> following changes:
>>
>>1. Use qemuBlockStorageSourceBuildJSONSocketAddress to build a single
>>   server address
>>2. Alter the output file to remove the server index "0." values
>>3. Alter the commit message appropriately as well.
>>
>> NB: If it's really desired I could resend the series, but I do ask that
>> someone else officially ACK's patch 1 since it's so large. I believe it
>> has to be moderated in... Although I could take Ashish's 1-6 testing as
>> as "good enough".
> 
> With all the squash-ins and .5 patches this got messy. Please repost.
> 

Sure, not problem. Any issue w/ me pushing patch 1 first?  The
2.10.replies and 2.10.xml? I think it's large enough that it needs
moderation...

John

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

Re: [libvirt] [PATCH v7 06.5/11] merge with previous...

[Peter Krempa]

On Thu, Sep 14, 2017 at 06:53:10 -0400, John Ferlan wrote:
> On 09/14/2017 04:13 AM, Peter Krempa wrote:
> > On Tue, Sep 12, 2017 at 14:39:58 -0400, John Ferlan wrote:

[...]

> > With all the squash-ins and .5 patches this got messy. Please repost.
> > 
> 
> Sure, not problem. Any issue w/ me pushing patch 1 first?  The
> 2.10.replies and 2.10.xml? I think it's large enough that it needs
> moderation...

I asked Pavel to have a look. It's fine from my point of view


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

[libvirt] [PATCH 5/7] qemu: Pass virArch * to virQEMUCapsCPUFilterFeatures

[Jiri Denemark]

The filter only needs to know the CPU architecture. Passing
virQEMUCapsPtr as opaque is a bit overkill.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c690cb3498..52d63f44ec 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3320,9 +3320,9 @@ static bool
 virQEMUCapsCPUFilterFeatures(const char *name,
  void *opaque)
 {
-virQEMUCapsPtr qemuCaps = opaque;
+virArch *arch = opaque;
 
-if (!ARCH_IS_X86(qemuCaps->arch))
+if (!ARCH_IS_X86(*arch))
 return true;
 
 if (STREQ(name, "cmt") ||
@@ -3534,7 +3534,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
 if (!hostCPU ||
 virCPUDefCopyModelFilter(cpu, hostCPU, true,
  virQEMUCapsCPUFilterFeatures,
- qemuCaps) < 0)
+ >arch) < 0)
 goto error;
 } else if (type == VIR_DOMAIN_VIRT_KVM &&
virCPUGetHostIsSupported(qemuCaps->arch)) {
-- 
2.14.1

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

Re: [libvirt] [PATCH v4 1/2] conf, docs: Add qcow2 cache configuration support

[Peter Krempa]

On Thu, Sep 14, 2017 at 16:37:03 +0800, Liu Qing wrote:
> On Thu, Sep 14, 2017 at 10:02:40AM +0200, Peter Krempa wrote:
> > On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:

[...]

> > One more thing. The design you've proposed is really not user friendly.
> > The user has to read a lenghty document, then inquire the parameters for
> > every single qcow2 image, do the calculations and set it in the XML.
> > This might be okay for higher level management tools, but not for users
> > who use libvirt directly.
> I agree this is not user friendly. I only give the user a choice, maybe
> not the best, right now. Any suggestion will be greatly appreicated.
> For our company, this feature will be an add-on to Openstack.

This means that you will have to deal with everything that I've pointed
out. How are you planning to expose this to the user? And which
calculations are you going to use?


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

Re: [libvirt] [PATCH v7 01/11] qemu: Add QEMU 2.10 x86_64 the generated capabilities

[Pavel Hrdina]

On Fri, Sep 01, 2017 at 01:09:45PM -0400, John Ferlan wrote:
> For reference, these were generated by updating a local qemu git
> repository to the latest upstream, making sure the latest dependencies
> were met via "dnf builddep qemu" from my sufficiently privileged root
> account, checking out the v2.10.0 tag, and building in order to generate
> an "x86_64-softmmu/qemu-system-x86_64" image.
> 
> Then using a clean libvirt tree updated to master and built, the image
> was then provided as input:
> 
> tests/qemucapsprobe /path/to/x86_64-softmmu/qemu-system-x86_64 > \
>tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies
> 
> With the .replies file in place and the DO_TEST line added and build,
> then running the following commands:
> 
> touch tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
> VIR_TEST_REGENERATE_OUTPUT=1 ./tests/qemucapabilitiestest
> 
> to generate tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml and both
> were added to the commit.
> 
> Signed-off-by: John Ferlan 
> ---
>  .../caps_2.10.0.x86_64.replies | 18144 
> +++
>  tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  |   798 +
>  tests/qemucapabilitiestest.c   | 1 +
>  3 files changed, 18943 insertions(+)
>  create mode 100644 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies
>  create mode 100644 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml

Capabilities generated by me are the same except for CPU features
because I have a different CPU.

Reviewed-by: Pavel Hrdina 


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

[libvirt] [PATCH 7/7] qemu: Filter CPU features returned by qemuConnectBaselineCPU

[Jiri Denemark]

The host CPU definitions reported in the capabilities XML may contain
CPU features unknown to QEMU, but the result of virConnectBaselineCPU is
supposed to be directly usable as a guest CPU definition and thus it
should only contain features QEMU knows about.

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

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e92c114f3e..e1a0dd553e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12990,6 +12990,7 @@ qemuConnectBaselineCPU(virConnectPtr conn 
ATTRIBUTE_UNUSED,
unsigned int flags)
 {
 virCPUDefPtr *cpus = NULL;
+virCPUDefPtr baseline = NULL;
 virCPUDefPtr cpu = NULL;
 char *cpustr = NULL;
 
@@ -13002,8 +13003,16 @@ qemuConnectBaselineCPU(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST)))
 goto cleanup;
 
-if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0,
-!!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE
+if (!(baseline = cpuBaseline(cpus, ncpus, NULL, 0,
+ !!(flags & 
VIR_CONNECT_BASELINE_CPU_MIGRATABLE
+goto cleanup;
+
+if (!(cpu = virCPUDefCopyWithoutModel(baseline)))
+goto cleanup;
+
+if (virCPUDefCopyModelFilter(cpu, baseline, false,
+ virQEMUCapsCPUFilterFeatures,
+ [0]->arch) < 0)
 goto cleanup;
 
 if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
@@ -13014,6 +13023,7 @@ qemuConnectBaselineCPU(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
  cleanup:
 virCPUDefListFree(cpus);
+virCPUDefFree(baseline);
 virCPUDefFree(cpu);
 
 return cpustr;
-- 
2.14.1

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

[libvirt] [PATCH 4/7] cpu: Drop cpuBaselineXML

[Jiri Denemark]

The implementation of virConnectBaselineCPU may be different for each
hypervisor. Thus it shouldn't really be implmented in the cpu code.

Signed-off-by: Jiri Denemark 
---
 src/bhyve/bhyve_driver.c | 22 +++---
 src/cpu/cpu.c| 58 
 src/cpu/cpu.h|  7 --
 src/libvirt_private.syms |  1 -
 src/libxl/libxl_driver.c | 22 +++---
 src/qemu/qemu_driver.c   | 22 +++---
 src/test/test_driver.c   | 22 +++---
 src/vz/vz_driver.c   | 22 +-
 8 files changed, 97 insertions(+), 79 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 3bcff88975..e8241f39ff 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1420,7 +1420,9 @@ bhyveConnectBaselineCPU(virConnectPtr conn,
 unsigned int ncpus,
 unsigned int flags)
 {
-char *cpu = NULL;
+virCPUDefPtr *cpus = NULL;
+virCPUDefPtr cpu = NULL;
+char *cpustr = NULL;
 
 virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
   VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
@@ -1428,10 +1430,24 @@ bhyveConnectBaselineCPU(virConnectPtr conn,
 if (virConnectBaselineCPUEnsureACL(conn) < 0)
 goto cleanup;
 
-cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags);
+if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST)))
+goto cleanup;
+
+if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0,
+!!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE
+goto cleanup;
+
+if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
+virCPUExpandFeatures(cpus[0]->arch, cpu) < 0)
+goto cleanup;
+
+cpustr = virCPUDefFormat(cpu, NULL, false);
 
  cleanup:
-return cpu;
+virCPUDefListFree(cpus);
+virCPUDefFree(cpu);
+
+return cpustr;
 }
 
 static int
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index e75f406040..a7c7c381b9 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -495,64 +495,6 @@ virCPUProbeHost(virArch arch)
 }
 
 
-/**
- * cpuBaselineXML:
- *
- * @xmlCPUs: list of host CPU XML descriptions
- * @ncpus: number of CPUs in @xmlCPUs
- * @models: list of CPU models that can be considered for the baseline CPU
- * @nmodels: number of CPU models in @models
- * @flags: bitwise-OR of virConnectBaselineCPUFlags
- *
- * Computes the most feature-rich CPU which is compatible with all given
- * host CPUs. If @models array is NULL, all models supported by libvirt will
- * be considered when computing the baseline CPU model, otherwise the baseline
- * CPU model will be one of the provided CPU @models.
- *
- * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt
- * will explicitly list all CPU features that are part of the host CPU,
- * without this flag features that are part of the CPU model will not be
- * listed.
- *
- * Returns XML description of the baseline CPU or NULL on error.
- */
-char *
-cpuBaselineXML(const char **xmlCPUs,
-   unsigned int ncpus,
-   const char **models,
-   unsigned int nmodels,
-   unsigned int flags)
-{
-virCPUDefPtr *cpus = NULL;
-virCPUDefPtr cpu = NULL;
-char *cpustr = NULL;
-
-VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels);
-
-virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
-  VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
-
-if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST)))
-goto cleanup;
-
-if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels,
-!!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE
-goto cleanup;
-
-if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
-virCPUExpandFeatures(cpus[0]->arch, cpu) < 0)
-goto cleanup;
-
-cpustr = virCPUDefFormat(cpu, NULL, false);
-
- cleanup:
-virCPUDefListFree(cpus);
-virCPUDefFree(cpu);
-
-return cpustr;
-}
-
-
 /**
  * cpuBaseline:
  *
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index c6ca111e97..5dda46ee70 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -196,13 +196,6 @@ virCPUGetHost(virArch arch,
 virCPUDefPtr
 virCPUProbeHost(virArch arch);
 
-char *
-cpuBaselineXML(const char **xmlCPUs,
-   unsigned int ncpus,
-   const char **models,
-   unsigned int nmodels,
-   unsigned int flags);
-
 virCPUDefPtr
 cpuBaseline (virCPUDefPtr *cpus,
  unsigned int ncpus,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 857e417f94..888e4e329b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1077,7 +1077,6 @@ virStoragePoolObjVolumeListExport;
 
 # cpu/cpu.h
 cpuBaseline;
-cpuBaselineXML;
 cpuDecode;
 cpuEncode;
 virCPUCheckFeature;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 

[libvirt] [PATCH 6/7] qemu: Publish virQEMUCapsCPUFilterFeatures

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 2 +-
 src/qemu/qemu_capabilities.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 52d63f44ec..7bc88be0f5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3316,7 +3316,7 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr 
qemuCaps,
 }
 
 
-static bool
+bool
 virQEMUCapsCPUFilterFeatures(const char *name,
  void *opaque)
 {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 85c390abf5..1400e885d9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -547,4 +547,7 @@ int virQEMUCapsFillDomainCaps(virCapsPtr caps,
 bool virQEMUCapsGuestIsNative(virArch host,
   virArch guest);
 
+bool virQEMUCapsCPUFilterFeatures(const char *name,
+  void *opaque);
+
 #endif /* __QEMU_CAPABILITIES_H__*/
-- 
2.14.1

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

[libvirt] [PATCH 3/7] cpu: Don't log CPU models in cpuBaselineXML

[Jiri Denemark]

They are logged in cpuBaseline anyway.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index bf3c6d53dd..e75f406040 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -526,18 +526,12 @@ cpuBaselineXML(const char **xmlCPUs,
 virCPUDefPtr *cpus = NULL;
 virCPUDefPtr cpu = NULL;
 char *cpustr = NULL;
-size_t i;
 
 VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels);
 
 virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
   VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
 
-if (models) {
-for (i = 0; i < nmodels; i++)
-VIR_DEBUG("models[%zu]=%s", i, NULLSTR(models[i]));
-}
-
 if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST)))
 goto cleanup;
 
-- 
2.14.1

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

Re: [libvirt] [PATCH v4 1/2] conf, docs: Add qcow2 cache configuration support

[Liu Qing]

On Thu, Sep 14, 2017 at 10:02:40AM +0200, Peter Krempa wrote:
> On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:
> > On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
> > > On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
> > > > On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
> > 
> > [...]
> > 
> > > > [1] There's discussion I can link to for other tuning parameters. The
> > > > gist is that allowing users to set something withoug giving them
> > > > guidance is pointless since they might not use it. Also if the guidance
> > > > is strict (e.g. a formula, libvirt or qemu should set the defaults
> > > > properly and not force users to do the calculation)
> > > The guidance could be found in doc/qcow2-cache.txt in qemu source code.
> > > As John Ferlan suggested I have added the file locaton in 
> > > formatdomain.html.in. 
> > 
> > Well, if the guidance is absolute (use this cache size and it's okay)
> > then we should implement it automatically (don't allow users
> > to set it.)
> 
> One more thing. The design you've proposed is really not user friendly.
> The user has to read a lenghty document, then inquire the parameters for
> every single qcow2 image, do the calculations and set it in the XML.
> This might be okay for higher level management tools, but not for users
> who use libvirt directly.
I agree this is not user friendly. I only give the user a choice, maybe
not the best, right now. Any suggestion will be greatly appreicated.
For our company, this feature will be an add-on to Openstack.

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

Re: [libvirt] [PATCH v7 06.5/11] merge with previous...

[Peter Krempa]

On Tue, Sep 12, 2017 at 14:39:58 -0400, John Ferlan wrote:
> This patch would be merged with the existing 06/11 patch for the
> following changes:
> 
>1. Use qemuBlockStorageSourceBuildJSONSocketAddress to build a single
>   server address
>2. Alter the output file to remove the server index "0." values
>3. Alter the commit message appropriately as well.
> 
> NB: If it's really desired I could resend the series, but I do ask that
> someone else officially ACK's patch 1 since it's so large. I believe it
> has to be moderated in... Although I could take Ashish's 1-6 testing as
> as "good enough".

With all the squash-ins and .5 patches this got messy. Please repost.


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

Re: [libvirt] [PATCH v4 1/2] conf, docs: Add qcow2 cache configuration support

[Liu Qing]

On Thu, Sep 14, 2017 at 09:29:28AM +0200, Peter Krempa wrote:
> On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
> > On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
> > > On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
> 
> [...]
> 
> > > [1] There's discussion I can link to for other tuning parameters. The
> > > gist is that allowing users to set something withoug giving them
> > > guidance is pointless since they might not use it. Also if the guidance
> > > is strict (e.g. a formula, libvirt or qemu should set the defaults
> > > properly and not force users to do the calculation)
> > The guidance could be found in doc/qcow2-cache.txt in qemu source code.
> > As John Ferlan suggested I have added the file locaton in 
> > formatdomain.html.in. 
> 
> Well, if the guidance is absolute (use this cache size and it's okay)
> then we should implement it automatically (don't allow users
> to set it.)
> 
> I'm asking whether there are some catches why not to do it automatically.
> E.g whether there's an possibility that users would do something *else*
> as described by the document and what would be the reasons for it.
I think there is some trade off between the cache size and performance.
Otherwise qemu does not need to export these parameters to user, they
could do the automation in qemu. The guidance only let the user know how
much disk space is coverred by caches, and how much memory will be
needed to cover all disk spaces. User should do their own decision to
choose a right size. But if the user is using the current version of
libvirt, they could not set the value even if they know what's the
proper value.

For example if the user may need a LUN which need high IOPS, he could
set the cache to cover all disk spaces. And in another situation, he
needs a large capacity,for example 4T, but low perfermance LUN, then he
could set the l2 cache size to a value much less than 512M.

> 
> One of the reasons might be memory consumption of the cache as it
> looks like you need 1 MiB of memory to fully cover a 8GiB image.
Yes, as I said above.
> 
> Also the problem is that those parameters are qemu-isms so we should be
> aware when modelling them in the XML since they can change and may not
> map well to any other technology.
Currently the new element is added as a child elemnt of driver, and the
driver type needs to be qcow2. Also add this kind information in the
doc.
> 
> Also how does the cache impact read-performance from the backing layers?
> We might need to set the cache for the backing layers as well.
I have a peek at the latest qemu code and the backing layers will have
the same cache size value as the top volume. And this will reduce metadata read
count if the table is already in the memory. Also more memory will
consumed.

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

Re: [libvirt] [PATCH v4 1/2] conf, docs: Add qcow2 cache configuration support

[Peter Krempa]

On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:
> On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
> > On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
> > > On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
> 
> [...]
> 
> > > [1] There's discussion I can link to for other tuning parameters. The
> > > gist is that allowing users to set something withoug giving them
> > > guidance is pointless since they might not use it. Also if the guidance
> > > is strict (e.g. a formula, libvirt or qemu should set the defaults
> > > properly and not force users to do the calculation)
> > The guidance could be found in doc/qcow2-cache.txt in qemu source code.
> > As John Ferlan suggested I have added the file locaton in 
> > formatdomain.html.in. 
> 
> Well, if the guidance is absolute (use this cache size and it's okay)
> then we should implement it automatically (don't allow users
> to set it.)

One more thing. The design you've proposed is really not user friendly.
The user has to read a lenghty document, then inquire the parameters for
every single qcow2 image, do the calculations and set it in the XML.
This might be okay for higher level management tools, but not for users
who use libvirt directly.


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

Re: [libvirt] [PATCH 5/5] qemu: Implement usernet address

[Michal Privoznik]

On 09/13/2017 06:02 PM, Laine Stump wrote:
> On 09/13/2017 06:46 AM, Michal Privoznik wrote:
>> On 09/12/2017 09:16 PM, Laine Stump wrote:
>>> On 09/12/2017 05:32 AM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1075520

 +/* QEMU needs some space to have
 + * some other 'hosts' on the network. */
 +if ((hasIPv4 && ip->prefix > 27) ||
 +(hasIPv6 && ip->prefix > 120)) {
 +virReportError(VIR_ERR_XML_ERROR, "%s",
 +   _("prefix too long"));
>>> You should also probably check for conflict with the addresses that are
>>> automatically used by slirp for DNS, default route, dhcp server, etc.
>>> (Although I *hate* needing to hard code stuff like that :-/)
>> Do I? If the prefix allows sufficiently enough room for them then the
>> defaults shouldn't clash. Or you mean checking against host's IPs? The
>> defaults according to the qemu docs are as follows:
>>
>> host = x.x.x.2
>> dhcpstart = x.x.x.15 - x.x.x.30
>> dns = x.x.x.3
>> smb = x.x.x.4
>>
> 
> Right. So let's say the user requests x.x.x.3 for the IP - that would
> conflict with the DNS server address.
> 
> I would really hate to have those specific checks hardcoded though (in
> case qemu changed the addresses for some reason. Maybe if we're lucky,
> qemu itself will error out if there is a conflict. If that's the case,
> then we can just leave it to them to report the error (as long as we're
> able to adequately scrape the error message so we can report something
> sensible).
> 

Ah, qemu does not report any error. They merely take the default netmask
for given IP address and zero out the subnet part. This is what I tried:


  
  
  
  


and this is how guest sees the interface:

3: ens8:  mtu 1500 qdisc fq_codel state UP 
group default qlen 1000
link/ether 00:11:22:33:44:55 brd ff:ff:ff:ff:ff:ff
inet 172.16.2.15/12 brd 172.31.255.255 scope global dynamic ens8
   valid_lft 86373sec preferred_lft 86373sec
inet6 2001:db8:ac10:fd01:a37e:8d3e:c189:d4f1/64 scope global noprefixroute 
dynamic 
   valid_lft 86374sec preferred_lft 14374sec


So, what if while parsing the IP address we would do the same? I mean,
zero out the subnet part of the address. If prefix was provided use that
otherwise use the default. Users are required to reload the domain XML
after any edit anyway. This also makes more sense IMO because users are
providing network address, not a host one.

Having said that, thanks for review on v2, but I'll postpone the push
until we have a clear agreement here.

Michal

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

Re: [libvirt] [PATCH v4 1/2] conf, docs: Add qcow2 cache configuration support

[Peter Krempa]

On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
> On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
> > On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:

[...]

> > [1] There's discussion I can link to for other tuning parameters. The
> > gist is that allowing users to set something withoug giving them
> > guidance is pointless since they might not use it. Also if the guidance
> > is strict (e.g. a formula, libvirt or qemu should set the defaults
> > properly and not force users to do the calculation)
> The guidance could be found in doc/qcow2-cache.txt in qemu source code.
> As John Ferlan suggested I have added the file locaton in 
> formatdomain.html.in. 

Well, if the guidance is absolute (use this cache size and it's okay)
then we should implement it automatically (don't allow users
to set it.)

I'm asking whether there are some catches why not to do it automatically.
E.g whether there's an possibility that users would do something *else*
as described by the document and what would be the reasons for it.

One of the reasons might be memory consumption of the cache as it
looks like you need 1 MiB of memory to fully cover a 8GiB image.

Also the problem is that those parameters are qemu-isms so we should be
aware when modelling them in the XML since they can change and may not
map well to any other technology.

Also how does the cache impact read-performance from the backing layers?
We might need to set the cache for the backing layers as well.


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