Re: [libvirt] [PATCH v7 0/9] Add setting CPU features (CPUID) with libxenlight driver.

[Jim Fehlig]

On 04/11/2018 07:03 PM, Marek Marczykowski-Górecki wrote:

Add support for CPUID setting based on  element. Since libxl format
support only adjusting specific bits over host CPU, only
mode='host-passthrough' is supported - other values are rejected (including
default 'custom'). This will break some configurations working before (bare
 element with for example NUMA configuration), but libxl driver never
supported full 'custom' mode - it was silently ignored, which might lead to
some unexpected effects.
Since mode='host-passthrough' is now necessary to specify CPU options, do not
enable nested HVM feature by mere presence of this element, require also
enabling it in libxl.conf. Nested HVM is still in "preview" state, so better be
explicit here.

v2 of this patch series:
https://www.redhat.com/archives/libvir-list/2017-July/msg00050.html

v3 of this patch series:
https://www.redhat.com/archives/libvir-list/2017-December/msg00314.html

v4 of this patch series:
https://www.redhat.com/archives/libvir-list/2018-February/msg00504.html

v5 of this patch series:
https://www.redhat.com/archives/libvir-list/2018-March/msg00796.html

v6 of this patch series:
https://www.redhat.com/archives/libvir-list/2018-March/msg01310.html

Marek Marczykowski-Górecki (9):
   libxl: fix libxlDriverConfigDispose for partially constructed object
   libxl: pass driver config to libxlMakeDomBuildInfo
   libxl: warn about ignored CPU mode=custom
   libxl: do not enable nested HVM unless global nested_hvm option enabled
   xenconfig: do not override def->cpu if already set elsewhere
   libxl: add support for CPUID features policy
   tests: check CPU features handling in libxl driver
   xenconfig: add CPUID handling to domXML <-> xl.cfg conversion
   tests: add test case for CPUID in xenconfig driver

  src/libxl/libvirtd_libxl.aug |   2 +-
  src/libxl/libxl.conf |   8 +-
  src/libxl/libxl_conf.c   |  66 +++-
  src/libxl/libxl_conf.h   |   6 +-
  src/libxl/libxl_domain.c |   2 +-
  src/libxl/test_libvirtd_libxl.aug.in |   1 +-
  src/xenconfig/xen_xl.c   | 236 ++--
  src/xenconfig/xen_xl.h   |   2 +-
  tests/libxlxml2domconfigdata/fullvirt-cpuid.json |  60 -
  tests/libxlxml2domconfigdata/fullvirt-cpuid.xml  |  34 ++-
  tests/libxlxml2domconfigtest.c   |  27 +-
  tests/virmocklibxl.c |  31 ++-
  tests/xlconfigdata/test-fullvirt-cpuid.cfg   |  25 ++-
  tests/xlconfigdata/test-fullvirt-cpuid.xml   |  36 ++-
  tests/xlconfigtest.c |   1 +-
  15 files changed, 492 insertions(+), 45 deletions(-)
  create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json
  create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
  create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.cfg
  create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.xml

base-commit: dffe584aa4194b0667924632e9e1ae12c5520956



Thanks a lot for the rebase. I fixed up patch 2 as we discussed and (finally) 
pushed the series!


Regards,
Jim

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

[libvirt] [PATCH] news: Xen: announce support for setting CPU features

[Jim Fehlig]

Signed-off-by: Jim Fehlig 
---
 docs/news.xml | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index a5c489151..dec3f134c 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -44,6 +44,18 @@
   add this controller when traditional PCI devices are in use.
 
   
+  
+
+  Xen: Support setting CPU features for host-passthrough model
+
+
+  The CPU model presented to Xen HVM domains is equivalent to libvirt's
+  host-passthrough model, although individual features can be enabled
+  and disabled via the cpuid setting. The libvirt libxl driver now
+  supports enabling and disabling individual features of the
+  host-passthrough CPU model.
+
+  
 
 
   
-- 
2.16.3

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

Re: [libvirt] [PATCH 2/2] storage: Check qemu-img encryption type capability

[John Ferlan]

On 04/17/2018 04:47 PM, Ján Tomko wrote:
> On Tue, Apr 17, 2018 at 03:23:33PM -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1526382
>>
>> As of QEMU 2.9, qemu-img has enforced using the "key-secret" for
>> creation of encrypted volumes. That is, LUKS encryption is now
>> required and the old (awful) qcow[2] encryption methodolgy is
>> no longer supported.
>>
>> In order to check for this, we scan the qemu-img -o help options
>> looking for "key-secret" and if set, we enforce during the create
>> volume processing that the about to be encrypted volume doesn't
>> attempt to use the old crufty encryption mechanism.
>>
>> Signed-off-by: John Ferlan 
>> ---
>> src/storage/storage_util.c | 32 ++--
>> 1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>> index 7df52239c2..d2e02a57ca 100644
>> --- a/src/storage/storage_util.c
>> +++ b/src/storage/storage_util.c
>> @@ -797,6 +797,7 @@ storagePloopResize(virStorageVolDefPtr vol,
>> enum {
>>     QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
>>     QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
>> +    QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET,
>> };
>>
>> static char *
>> @@ -824,6 +825,14 @@ virStorageBackendQemuImgSupportsCompat(const char
>> *output)
>>     return strstr(output, "\ncompat ");
>> }
>>
>> +
>> +static bool
>> +virStorageBackendQemuImgRequiresKeySecret(const char *output)
>> +{
>> +    return strstr(output, "key-secret");
>> +}
>> +
>> +
> 
> NACK
> 
> adding more -help output scraping just for a nicer error message for a
> feature noone should be using in the first place is not worth it.
> 
> Jano


Fair enough - considering your other series...

As a consumer would you expect an error message for any create using
qcow[2] then?

Or should we just rip out the qcow[2] encryption stuff too?

IDC, either way.  It's the delta then between qemu 1.5 and 2.9 to
consider...

John

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

[libvirt] [qemu RFC v2] qapi: add "firmware.json"

[Laszlo Ersek]

Add a schema that describes the different uses and properties of virtual
machine firmware.

Each firmware executable installed on a host system should come with at
least one JSON file that conforms to this schema. Each file informs the
management applications about the firmware's properties and one possible
use case / feature set.

In addition, a configuration directory with symlinks to the JSON files
should exist, with the symlinks carefully named to reflect a priority
order. Management applications can then search this directory in priority
order for the first firmware description that satisfies their search
criteria. The found JSON file provides the management layer with domain
configuration bits that are required to run the firmware binary for a
certain use case or feature set.

Cc: "Daniel P. Berrange" 
Cc: Alexander Graf 
Cc: Ard Biesheuvel 
Cc: David Gibson 
Cc: Eric Blake 
Cc: Gary Ching-Pang Lin 
Cc: Gerd Hoffmann 
Cc: Kashyap Chamarthy 
Cc: Markus Armbruster 
Cc: Michael Roth 
Cc: Michal Privoznik 
Cc: Paolo Bonzini 
Cc: Peter Krempa 
Cc: Peter Maydell 
Cc: Thomas Huth 
Signed-off-by: Laszlo Ersek 
---

Notes:
RFCv2:
- previous version (RFCv1) was posted at
  <20180407000117.25640-1-lersek@redhat.com">http://mid.mail-archive.com/20180407000117.25640-1-lersek@redhat.com>

- v2 is basically a rewrite from scratch, starting out with Dan's
  definitions from
  <20180410102033.GL5155@redhat.com">http://mid.mail-archive.com/20180410102033.GL5155@redhat.com> and
  <20180410110357.GP5155@redhat.com">http://mid.mail-archive.com/20180410110357.GP5155@redhat.com>,
  hopefully addressing others' feedback as well

RFCv1:
- Folks on the CC list, please try to see if the suggested schema is
  flexible enough to describe the virtual firmware(s) that you are
  familiar with. Thanks!

 Makefile  |   9 +
 Makefile.objs |   4 +
 qapi/firmware.json| 503 ++
 qapi/qapi-schema.json |   1 +
 qmp.c |   5 +
 .gitignore|   4 +
 6 files changed, 526 insertions(+)
 create mode 100644 qapi/firmware.json

diff --git a/Makefile b/Makefile
index d71dd5bea416..32034abe1583 100644
--- a/Makefile
+++ b/Makefile
@@ -97,6 +97,7 @@ GENERATED_FILES += qapi/qapi-types-block.h 
qapi/qapi-types-block.c
 GENERATED_FILES += qapi/qapi-types-char.h qapi/qapi-types-char.c
 GENERATED_FILES += qapi/qapi-types-common.h qapi/qapi-types-common.c
 GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c
+GENERATED_FILES += qapi/qapi-types-firmware.h qapi/qapi-types-firmware.c
 GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c
 GENERATED_FILES += qapi/qapi-types-migration.h qapi/qapi-types-migration.c
 GENERATED_FILES += qapi/qapi-types-misc.h qapi/qapi-types-misc.c
@@ -115,6 +116,7 @@ GENERATED_FILES += qapi/qapi-visit-block.h 
qapi/qapi-visit-block.c
 GENERATED_FILES += qapi/qapi-visit-char.h qapi/qapi-visit-char.c
 GENERATED_FILES += qapi/qapi-visit-common.h qapi/qapi-visit-common.c
 GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c
+GENERATED_FILES += qapi/qapi-visit-firmware.h qapi/qapi-visit-firmware.c
 GENERATED_FILES += qapi/qapi-visit-introspect.h qapi/qapi-visit-introspect.c
 GENERATED_FILES += qapi/qapi-visit-migration.h qapi/qapi-visit-migration.c
 GENERATED_FILES += qapi/qapi-visit-misc.h qapi/qapi-visit-misc.c
@@ -132,6 +134,7 @@ GENERATED_FILES += qapi/qapi-commands-block.h 
qapi/qapi-commands-block.c
 GENERATED_FILES += qapi/qapi-commands-char.h qapi/qapi-commands-char.c
 GENERATED_FILES += qapi/qapi-commands-common.h qapi/qapi-commands-common.c
 GENERATED_FILES += qapi/qapi-commands-crypto.h qapi/qapi-commands-crypto.c
+GENERATED_FILES += qapi/qapi-commands-firmware.h qapi/qapi-commands-firmware.c
 GENERATED_FILES += qapi/qapi-commands-introspect.h 
qapi/qapi-commands-introspect.c
 GENERATED_FILES += qapi/qapi-commands-migration.h 
qapi/qapi-commands-migration.c
 GENERATED_FILES += qapi/qapi-commands-misc.h qapi/qapi-commands-misc.c
@@ -149,6 +152,7 @@ GENERATED_FILES += qapi/qapi-events-block.h 
qapi/qapi-events-block.c
 GENERATED_FILES += qapi/qapi-events-char.h qapi/qapi-events-char.c
 GENERATED_FILES += qapi/qapi-events-common.h qapi/qapi-events-common.c
 GENERATED_FILES += qapi/qapi-events-crypto.h qapi/qapi-events-crypto.c
+GENERATED_FILES += qapi/qapi-events-firmware.h qapi/qapi-events-firmware.c
 GENERATED_FILES += qapi/qapi-events-introspect.h qapi/qapi-events-introspect.c
 GENERATED_FILES += qapi/qapi-events-migration.h qapi/qapi-events-migration.c
 GENERATED_FILES += qapi/qapi-events-misc.h 

Re: [libvirt] [PATCH v7 2/9] libxl: pass driver config to libxlMakeDomBuildInfo

[Jim Fehlig]

On 04/17/2018 04:22 PM, Marek Marczykowski-Górecki wrote:

On Tue, Apr 17, 2018 at 04:04:01PM -0600, Jim Fehlig wrote:

On 04/17/2018 02:20 PM, Marek Marczykowski-Górecki wrote:

On Tue, Apr 17, 2018 at 02:00:25PM -0600, Jim Fehlig wrote:

Your response in the V6 thread about "conflicting types (with
libxl_get_free_memory_0x040700)" was a good hint. Things work fine for me on
Xen 4.4 through 4.10 with the following squashed in

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2c0d1311c..8c4b6c220 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -528,6 +528,7 @@ libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) 
$(LIBXML_LIBS)

   virmocklibxl_la_SOURCES = \
  virmocklibxl.c
+virmocklibxl_la_CFLAGS = $(LIBXL_CFLAGS)
   virmocklibxl_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
   virmocklibxl_la_LIBADD = $(MOCKLIBS_LIBS)

But I'm still confused as to how this works for you. Any idea about that? :-)


Oh, I see it now. For me, -DLIBXL_API_VERSION=0x040400 landed in CFLAGS,
not only LIBXL_CFLAGS. According to config.log it happened just after
testing for xenlight - first, the test using pkg-config failed (something
wrong with my libxl installation), but then fallback to manual check.
And since then, every gcc call have -DLIBXL_API_VERSION=0x040400.

Looking at m4/virt-driver-libxl.m4, I think it's because saved
old_CFLAGS is overridden by LIBVIRT_CHECK_LIB with actual CFLAGS (now
containing -DLIBXL_API_VERSION=0x040400).


Ah, looks to be the case. But that is existing and can be fixed in a
followup IMO. So do you agree with the change to add LIBXL_CFLAGS to
virmocklibxl_la_CFLAGS?


Yes, of course. Should I submit yet another version?


No, that's fine. I'll take care of it when pushing the series.

Regards,
Jim

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

Re: [libvirt] [PATCH v7 2/9] libxl: pass driver config to libxlMakeDomBuildInfo

[Marek Marczykowski-Górecki]

On Tue, Apr 17, 2018 at 04:04:01PM -0600, Jim Fehlig wrote:
> On 04/17/2018 02:20 PM, Marek Marczykowski-Górecki wrote:
> > On Tue, Apr 17, 2018 at 02:00:25PM -0600, Jim Fehlig wrote:
> > > Your response in the V6 thread about "conflicting types (with
> > > libxl_get_free_memory_0x040700)" was a good hint. Things work fine for me 
> > > on
> > > Xen 4.4 through 4.10 with the following squashed in
> > > 
> > > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > > index 2c0d1311c..8c4b6c220 100644
> > > --- a/tests/Makefile.am
> > > +++ b/tests/Makefile.am
> > > @@ -528,6 +528,7 @@ libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) 
> > > $(LIBXML_LIBS)
> > > 
> > >   virmocklibxl_la_SOURCES = \
> > >  virmocklibxl.c
> > > +virmocklibxl_la_CFLAGS = $(LIBXL_CFLAGS)
> > >   virmocklibxl_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
> > >   virmocklibxl_la_LIBADD = $(MOCKLIBS_LIBS)
> > > 
> > > But I'm still confused as to how this works for you. Any idea about that? 
> > > :-)
> > 
> > Oh, I see it now. For me, -DLIBXL_API_VERSION=0x040400 landed in CFLAGS,
> > not only LIBXL_CFLAGS. According to config.log it happened just after
> > testing for xenlight - first, the test using pkg-config failed (something
> > wrong with my libxl installation), but then fallback to manual check.
> > And since then, every gcc call have -DLIBXL_API_VERSION=0x040400.
> > 
> > Looking at m4/virt-driver-libxl.m4, I think it's because saved
> > old_CFLAGS is overridden by LIBVIRT_CHECK_LIB with actual CFLAGS (now
> > containing -DLIBXL_API_VERSION=0x040400).
> 
> Ah, looks to be the case. But that is existing and can be fixed in a
> followup IMO. So do you agree with the change to add LIBXL_CFLAGS to
> virmocklibxl_la_CFLAGS?

Yes, of course. Should I submit yet another version?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


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

Re: [libvirt] [dbus PATCH 04/25] Implement BlockPeek metohd for Domain Interface

[Ján Tomko]

s/metohd/method/ in the commit summary

On Tue, Apr 17, 2018 at 04:08:49PM +0200, Ján Tomko wrote:

On Tue, Apr 17, 2018 at 02:04:23PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  9 +
src/domain.c| 39 +++
2 files changed, 48 insertions(+)



Jano


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

Re: [libvirt] [dbus PATCH 12/25] Implement CoreDumpWithFormat method for Domain Interface

[Ján Tomko]

On Tue, Apr 17, 2018 at 02:04:31PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  7 +++
src/domain.c| 26 ++
2 files changed, 33 insertions(+)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index c7fb637..05f4da9 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -106,6 +106,13 @@
  
  

+
+  https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCoreDumpWithFormat"/>
+  
+  
+  
+


Analogically to the *Flags APIs, this D-Bus method can be called just "CoreDump"
since there is no legacy method with that name.

Reviewed-by: Ján Tomko 

Jano


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

[libvirt] [PATCH v2 8/9] uml: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/uml/uml_driver.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index ac168ce77..56dfd7b58 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -2584,11 +2584,8 @@ umlDomainOpenConsole(virDomainPtr dom,
 if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (dev_name) {
 for (i = 0; i < vm->def->nconsoles; i++) {
-- 
2.17.0

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

[libvirt] [PATCH v2 7/9] openvz: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/openvz/openvz_driver.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 9900e8bab..66e589313 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -575,11 +575,8 @@ static int openvzDomainSuspend(virDomainPtr dom)
 if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
 return -1;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
 openvzSetProgramSentinal(prog, vm->def->name);
@@ -605,11 +602,8 @@ static int openvzDomainResume(virDomainPtr dom)
 if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
 return -1;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
 openvzSetProgramSentinal(prog, vm->def->name);
@@ -1895,11 +1889,8 @@ openvzDomainInterfaceStats(virDomainPtr dom,
 if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
 return -1;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (!(net = virDomainNetFind(vm->def, device)))
 goto cleanup;
@@ -2135,11 +2126,8 @@ openvzDomainMigrateBegin3Params(virDomainPtr domain,
 if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
 return NULL;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (openvzGetVEStatus(vm, , NULL) == -1)
 goto cleanup;
-- 
2.17.0

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

[libvirt] [PATCH v2 2/9] qemu: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/qemu/qemu_domain.c |   5 +-
 src/qemu/qemu_driver.c | 271 +
 2 files changed, 56 insertions(+), 220 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7000de6a9..decbdb004 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8678,11 +8678,8 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr 
driver,
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
 return -1;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorGetBalloonInfo(priv->mon, );
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fcd79bd71..a3c806271 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1837,11 +1837,8 @@ static int qemuDomainSuspend(virDomainPtr dom)
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_SUSPEND) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
 reason = VIR_DOMAIN_PAUSED_MIGRATION;
@@ -1906,11 +1903,8 @@ static int qemuDomainResume(virDomainPtr dom)
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 state = virDomainObjGetState(vm, );
 if (state == VIR_DOMAIN_PMSUSPENDED) {
@@ -2090,11 +2084,8 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
 useAgent = false;
 }
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (useAgent) {
 qemuAgentPtr agent;
@@ -2157,11 +2148,8 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags)
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 priv = vm->privateData;
 qemuDomainObjEnterMonitor(driver, vm);
@@ -,11 +2210,8 @@ qemuDomainDestroyFlags(virDomainPtr dom,
 if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 state = virDomainObjGetState(vm, );
 starting = (state == VIR_DOMAIN_PAUSED &&
@@ -2541,11 +2526,8 @@ static int qemuDomainInjectNMI(virDomainPtr domain, 
unsigned int flags)
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorInjectNMI(priv->mon);
@@ -2604,11 +2586,8 @@ static int qemuDomainSendKey(virDomainPtr domain,
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes);
@@ -2721,11 +2700,8 @@ qemuDomainGetControlInfo(virDomainPtr dom,
 if (virDomainGetControlInfoEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 priv = vm->privateData;
 
@@ -3537,11 +3513,8 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, 
const char *dxml,
 if (virDomainSaveFlagsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-

[libvirt] [PATCH v2 5/9] bhyve: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/bhyve/bhyve_driver.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 24c4a9c80..8aff0c65c 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -883,11 +883,8 @@ bhyveDomainCreateWithFlags(virDomainPtr dom,
 if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is already running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 ret = virBhyveProcessStart(dom->conn, privconn, vm,
VIR_DOMAIN_RUNNING_BOOTED,
@@ -996,11 +993,8 @@ bhyveDomainDestroy(virDomainPtr dom)
 if (virDomainDestroyEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
 event = virDomainEventLifecycleNewFromObj(vm,
@@ -1031,11 +1025,8 @@ bhyveDomainShutdown(virDomainPtr dom)
 if (virDomainShutdownEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 ret = virBhyveProcessShutdown(vm);
 
@@ -1062,11 +1053,8 @@ bhyveDomainOpenConsole(virDomainPtr dom,
 if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (!vm->def->nserials) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-- 
2.17.0

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

[libvirt] [PATCH v2 6/9] lxc: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/lxc/lxc_driver.c | 60 +---
 1 file changed, 12 insertions(+), 48 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 4097cef93..008e41bda 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1508,11 +1508,8 @@ lxcDomainDestroyFlags(virDomainPtr dom,
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 priv = vm->privateData;
 ret = virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
@@ -2382,11 +2379,8 @@ lxcDomainBlockStats(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -2468,11 +2462,8 @@ lxcDomainBlockStatsFlags(virDomainPtr dom,
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -2876,11 +2867,8 @@ lxcDomainInterfaceStats(virDomainPtr dom,
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (!(net = virDomainNetFind(vm->def, device)))
 goto endjob;
@@ -3100,11 +3088,8 @@ static int lxcDomainSuspend(virDomainPtr dom)
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
 if (lxcFreezeContainer(vm) < 0) {
@@ -3155,11 +3140,8 @@ static int lxcDomainResume(virDomainPtr dom)
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 state = virDomainObjGetState(vm, NULL);
 if (state == VIR_DOMAIN_RUNNING) {
@@ -3214,11 +3196,8 @@ lxcDomainOpenConsole(virDomainPtr dom,
 if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (dev_name) {
 for (i = 0; i < vm->def->nconsoles; i++) {
@@ -3292,11 +3271,8 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 /*
  * XXX if the kernel has /proc/$PID/ns/pid we can
@@ -3391,11 +3367,8 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (priv->initpid == 0) {
 virReportError(VIR_ERR_OPERATION_INVALID,
@@ -3474,11 +3447,8 @@ lxcDomainReboot(virDomainPtr dom,
 if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (priv->initpid == 0) {
 

[libvirt] [PATCH v2 1/9] Add function that raises error if domain is not active

[Clementine Hayat]

Add a function named virDomainObjCheckIsActive in src/conf/domain_conf.c.
It calls virDomainObjIsActive, raises error if necessary and returns.

There is a lot of occurence of this pattern and it will save 3 lines on
each call.

Signed-off-by: Clementine Hayat 
---
 src/conf/domain_conf.c   | 11 +++
 src/conf/domain_conf.h   |  2 ++
 src/libvirt_private.syms |  1 +
 3 files changed, 14 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d23182f18..dadb63360 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6003,6 +6003,17 @@ virDomainDefValidate(virDomainDefPtr def,
 return 0;
 }
 
+int
+virDomainObjCheckActive(virDomainObjPtr dom)
+{
+if (!virDomainObjIsActive(dom)) {
+   virReportError(VIR_ERR_OPERATION_INVALID,
+  "%s", _("domain is not running"));
+   return -1;
+}
+return 0;
+}
+
 
 /**
  * virDomainDeviceLoadparmIsValid
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bbaa24137..122a051b2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2722,6 +2722,8 @@ virDomainObjIsActive(virDomainObjPtr dom)
 return dom->def->id != -1;
 }
 
+int virDomainObjCheckActive(virDomainObjPtr dom);
+
 int virDomainDefSetVcpusMax(virDomainDefPtr def,
 unsigned int vcpus,
 virDomainXMLOptionPtr xmlopt);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cab324c4d..99b5a0235 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -474,6 +474,7 @@ virDomainNostateReasonTypeFromString;
 virDomainNostateReasonTypeToString;
 virDomainObjAssignDef;
 virDomainObjBroadcast;
+virDomainObjCheckActive;
 virDomainObjCopyPersistentDef;
 virDomainObjEndAPI;
 virDomainObjFormat;
-- 
2.17.0

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

[libvirt] [PATCH v2 0/9] Add function that raises error if domain is not active

[Clementine Hayat]

This is my GSOC patch contribution.

This change was suggested on BiteSizedTasks in the libvirt wiki[1].

in libvirt there is lots of occurences of this same pattern:

if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
   "%s", _("domain is not running"));
goto out;
}

This series replace these calls with a new function that check if the
domain is active and log directly the error. This allows to remove
almost 300 lines of code in the code base.

[1] 
https://wiki.libvirt.org/page/BiteSizedTasks#Add_function_that_raises_error_if_domain_is_not_active

Changes since v2:
* renamed virDomainObjCheckIsActive into virDomainObjCheckActive
* add the remaining occurences

Clementine Hayat (9):
  Add function that raises error if domain is not active
  qemu: start using virDomainObjCheckActive
  test: start using virDomainObjCheckActive
  libxl: start using virDomainObjCheckActive
  bhyve: start using virDomainObjCheckActive
  lxc: start using virDomainObjCheckActive
  openvz: start using virDomainObjCheckActive
  uml: start using virDomainObjCheckActive
  vz: start using virDomainObjCheckActive

 src/bhyve/bhyve_driver.c   |  20 +--
 src/conf/domain_conf.c |  11 ++
 src/conf/domain_conf.h |   2 +
 src/libvirt_private.syms   |   1 +
 src/libxl/libxl_driver.c   |  97 +++--
 src/lxc/lxc_driver.c   |  60 ++--
 src/openvz/openvz_driver.c |  20 +--
 src/qemu/qemu_domain.c |   5 +-
 src/qemu/qemu_driver.c | 271 -
 src/test/test_driver.c |  35 +
 src/uml/uml_driver.c   |   5 +-
 src/vz/vz_driver.c |   5 +-
 12 files changed, 120 insertions(+), 412 deletions(-)

-- 
2.17.0

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

[libvirt] [PATCH v2 4/9] libxl: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/libxl/libxl_driver.c | 97 +---
 1 file changed, 21 insertions(+), 76 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8808da8db..b66a1de5f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1164,10 +1164,8 @@ libxlDomainSuspend(virDomainPtr dom)
 if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
 if (libxl_domain_pause(cfg->ctx, vm->def->id) != 0) {
@@ -1220,10 +1218,8 @@ libxlDomainResume(virDomainPtr dom)
 if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
 if (libxl_domain_unpause(cfg->ctx, vm->def->id) != 0) {
@@ -1278,11 +1274,8 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int 
flags)
 if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (flags & VIR_DOMAIN_SHUTDOWN_PARAVIRT) {
 ret = libxl_domain_shutdown(cfg->ctx, vm->def->id);
@@ -1344,11 +1337,8 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
 if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) {
 ret = libxl_domain_reboot(cfg->ctx, vm->def->id);
@@ -1390,11 +1380,8 @@ libxlDomainDestroyFlags(virDomainPtr dom,
 if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (libxlDomainDestroyInternal(driver, vm) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1797,10 +1784,8 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, 
const char *dxml,
 if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (libxlDoDomainSave(driver, vm, to) < 0)
 goto endjob;
@@ -1925,10 +1910,8 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, 
unsigned int flags)
 if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 
 if (!(flags & VIR_DUMP_LIVE) &&
 virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
@@ -2022,10 +2005,8 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int 
flags)
 if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
-}
 if (!vm->persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot do managed save for transient domain"));
@@ -2493,10 +2474,8 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr 
info, int maxinfo,
 if (virDomainGetVcpusEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, ,
 )) == NULL) {
@@ -4466,10 +4445,8 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int 
*nparams)
 if (virDomainGetSchedulerTypeEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
-

[libvirt] [PATCH v2 3/9] test: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/test/test_driver.c | 35 +++
 1 file changed, 7 insertions(+), 28 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index eec7a8292..43221e547 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1779,11 +1779,8 @@ static int testDomainDestroyFlags(virDomainPtr domain,
 if (!(privdom = testDomObjFromDomain(domain)))
 goto cleanup;
 
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
 goto cleanup;
-}
 
 testDomainShutdownState(domain, privdom, VIR_DOMAIN_SHUTOFF_DESTROYED);
 event = virDomainEventLifecycleNewFromObj(privdom,
@@ -1921,11 +1918,8 @@ static int testDomainReboot(virDomainPtr domain,
 if (!(privdom = testDomObjFromDomain(domain)))
 goto cleanup;
 
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
 goto cleanup;
-}
 
 virDomainObjSetState(privdom, VIR_DOMAIN_SHUTDOWN,
  VIR_DOMAIN_SHUTDOWN_USER);
@@ -2049,11 +2043,8 @@ testDomainSaveFlags(virDomainPtr domain, const char 
*path,
 if (!(privdom = testDomObjFromDomain(domain)))
 goto cleanup;
 
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
 goto cleanup;
-}
 
 xml = virDomainDefFormat(privdom->def, privconn->caps,
  VIR_DOMAIN_DEF_FORMAT_SECURE);
@@ -2255,11 +2246,8 @@ static int testDomainCoreDumpWithFormat(virDomainPtr 
domain,
 if (!(privdom = testDomObjFromDomain(domain)))
 goto cleanup;
 
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
 goto cleanup;
-}
 
 if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
 virReportSystemError(errno,
@@ -3231,11 +3219,8 @@ static int testDomainBlockStats(virDomainPtr domain,
 if (!(privdom = testDomObjFromDomain(domain)))
 return ret;
 
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
 goto error;
-}
 
 if (virDomainDiskIndexByName(privdom->def, path, false) < 0) {
 virReportError(VIR_ERR_INVALID_ARG,
@@ -3278,11 +3263,8 @@ testDomainInterfaceStats(virDomainPtr domain,
 if (!(privdom = testDomObjFromDomain(domain)))
 return -1;
 
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
 goto error;
-}
 
 if (!(net = virDomainNetFind(privdom->def, device)))
 goto error;
@@ -5962,11 +5944,8 @@ testDomainManagedSave(virDomainPtr dom, unsigned int 
flags)
 if (!(vm = testDomObjFromDomain(dom)))
 return -1;
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
 goto cleanup;
-}
 
 if (!vm->persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-- 
2.17.0

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

[libvirt] [PATCH v2 9/9] vz: start using virDomainObjCheckActive

[Clementine Hayat]

Signed-off-by: Clementine Hayat 
---
 src/vz/vz_driver.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index e51d968f2..3094afccb 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3998,11 +3998,8 @@ vzDomainBlockResize(virDomainPtr domain,
 if (vzEnsureDomainExists(dom) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(dom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (virDomainObjCheckActive(dom) < 0)
 goto cleanup;
-}
 
 if (!(disk = virDomainDiskByName(dom->def, path, false))) {
 virReportError(VIR_ERR_INVALID_ARG,
-- 
2.17.0

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

Re: [libvirt] [PATCH v7 2/9] libxl: pass driver config to libxlMakeDomBuildInfo

[Jim Fehlig]

On 04/17/2018 02:20 PM, Marek Marczykowski-Górecki wrote:

On Tue, Apr 17, 2018 at 02:00:25PM -0600, Jim Fehlig wrote:

Your response in the V6 thread about "conflicting types (with
libxl_get_free_memory_0x040700)" was a good hint. Things work fine for me on
Xen 4.4 through 4.10 with the following squashed in

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2c0d1311c..8c4b6c220 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -528,6 +528,7 @@ libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) 
$(LIBXML_LIBS)

  virmocklibxl_la_SOURCES = \
 virmocklibxl.c
+virmocklibxl_la_CFLAGS = $(LIBXL_CFLAGS)
  virmocklibxl_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
  virmocklibxl_la_LIBADD = $(MOCKLIBS_LIBS)

But I'm still confused as to how this works for you. Any idea about that? :-)


Oh, I see it now. For me, -DLIBXL_API_VERSION=0x040400 landed in CFLAGS,
not only LIBXL_CFLAGS. According to config.log it happened just after
testing for xenlight - first, the test using pkg-config failed (something
wrong with my libxl installation), but then fallback to manual check.
And since then, every gcc call have -DLIBXL_API_VERSION=0x040400.

Looking at m4/virt-driver-libxl.m4, I think it's because saved
old_CFLAGS is overridden by LIBVIRT_CHECK_LIB with actual CFLAGS (now
containing -DLIBXL_API_VERSION=0x040400).


Ah, looks to be the case. But that is existing and can be fixed in a followup 
IMO. So do you agree with the change to add LIBXL_CFLAGS to virmocklibxl_la_CFLAGS?


Regards,
Jim

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

[libvirt] [PATCH 3/3] storage: remove qemu-img help scraping

[Ján Tomko]

We have been checking whether qemu-img supports the -o compat
option by scraping the -help output.

Since we require QEMU 1.5.0 now and this option was introduced in 1.1,
assume we support it and ditch the help parsing code along with the
extra qemu-img invocation.

Signed-off-by: Ján Tomko 
---
 src/storage/storage_util.c | 73 +++---
 src/storage/storage_util.h |  1 -
 tests/storagevolxml2argvtest.c |  5 ++-
 3 files changed, 6 insertions(+), 73 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 897dfdaae..f7a4231e2 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -787,61 +787,6 @@ storagePloopResize(virStorageVolDefPtr vol,
 return ret;
 }
 
-/* Flag values shared w/ storagevolxml2argvtest.c.
- *
- * QEMU_IMG_BACKING_FORMAT_OPTIONS (added in qemu 0.11)
- * QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT
- *was made necessary due to 2.0 change to change the default
- *qcow2 file format from 0.10 to 1.1.
- */
-enum {
-QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
-QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
-};
-
-static bool
-virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
-{
-bool ret = false;
-char *output;
-virCommandPtr cmd = NULL;
-
-cmd = virCommandNewArgList(qemuimg, "create", "-o", "?", "-f", "qcow2",
-   "/dev/null", NULL);
-
-virCommandAddEnvString(cmd, "LC_ALL=C");
-virCommandSetOutputBuffer(cmd, );
-
-if (virCommandRun(cmd, NULL) < 0)
-goto cleanup;
-
-if (strstr(output, "\ncompat "))
-ret = true;
-
- cleanup:
-virCommandFree(cmd);
-VIR_FREE(output);
-return ret;
-}
-
-
-static int
-virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
-{
-/* As of QEMU 0.11 the [-o options] support was added via qemu
- * commit id '9ea2ea71', so we start with that base and figure
- * out what else we have */
-int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
-
-/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
- * understands. Since we still support QEMU 0.12 and newer, we need
- * to be able to handle the previous format as can be set via a
- * compat=0.10 option. */
-if (virStorageBackendQemuImgSupportsCompat(qemuimg))
-ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
-
-return ret;
-}
 
 /* The _virStorageBackendQemuImgInfo separates the command line building from
  * the volume definition so that qemuDomainSnapshotCreateInactiveExternal can
@@ -1089,14 +1034,12 @@ 
storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
 
 static int
 storageBackendCreateQemuImgSetOptions(virCommandPtr cmd,
-  int imgformat,
   virStorageEncryptionInfoDefPtr enc,
   struct _virStorageBackendQemuImgInfo 
info)
 {
 char *opts = NULL;
 
-if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat &&
-imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT)
+if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat)
 info.compat = "0.10";
 
 if (storageBackendCreateQemuImgOpts(enc, , info) < 0)
@@ -1170,16 +1113,13 @@ storageBackendResizeQemuImgImageOpts(virCommandPtr cmd,
 }
 
 
-/* Create a qemu-img virCommand from the supplied binary path,
- * volume definitions and imgformat
- */
+/* Create a qemu-img virCommand from the supplied arguments */
 virCommandPtr
 virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
  virStorageVolDefPtr vol,
  virStorageVolDefPtr inputvol,
  unsigned int flags,
  const char *create_tool,
- int imgformat,
  const char *secretPath)
 {
 virCommandPtr cmd = NULL;
@@ -1293,7 +1233,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 enc = >target.encryption->encinfo;
 }
 
-if (storageBackendCreateQemuImgSetOptions(cmd, imgformat, enc, info) < 0)
+if (storageBackendCreateQemuImgSetOptions(cmd, enc, info) < 0)
 goto error;
 VIR_FREE(info.secretAlias);
 
@@ -1386,7 +1326,6 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 {
 int ret = -1;
 char *create_tool;
-int imgformat;
 virCommandPtr cmd;
 char *secretPath = NULL;
 
@@ -1400,10 +1339,6 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 return -1;
 }
 
-imgformat = virStorageBackendQEMUImgBackingFormat(create_tool);
-if (imgformat < 0)
-goto cleanup;
-
 if (vol->target.format == VIR_STORAGE_FILE_RAW &&
 vol->target.encryption &&
 vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
@@ -1414,7 +1349,7 @@ 

[libvirt] [PATCH 2/3] tests: assume FMT_COMPAT for qemu-img tests

[Ján Tomko]

No point in testing outdated command lines.

Signed-off-by: Ján Tomko 
---
 .../qcow2-nocapacity-convert-prealloc.argv |  2 +-
 tests/storagevolxml2argvdata/qcow2-nocapacity.argv |  2 +-
 tests/storagevolxml2argvtest.c | 32 +++---
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git 
a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv 
b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
index b151b9401..73499178e 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
@@ -1,4 +1,4 @@
 qemu-img convert -f raw -O qcow2 \
--o encryption=on,preallocation=falloc \
+-o encryption=on,preallocation=falloc,compat=0.10 \
 /var/lib/libvirt/images/sparse.img \
 /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv 
b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv
index 1198cbaf2..fd8805589 100644
--- a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv
+++ b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv
@@ -1,5 +1,5 @@
 qemu-img create \
 -f qcow2 \
 -b /dev/null \
--o backing_fmt=raw,encryption=on \
+-o backing_fmt=raw,encryption=on,compat=0.10 \
 /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 4353ad467..68ee9c3d8 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -180,10 +180,10 @@ mymain(void)
 unsigned int flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 
 #define DO_TEST_FULL(shouldFail, parseflags, pool, vol, inputpool, inputvol, \
- cmdline, flags, imgformat) \
+ cmdline, flags) \
 do { \
 struct testInfo info = { shouldFail, pool, vol, inputpool, inputvol, \
- cmdline, flags, imgformat, parseflags }; \
+ cmdline, flags, FMT_COMPAT, parseflags }; \
 if (virTestRun("Storage Vol XML-2-argv " cmdline, \
testCompareXMLToArgvHelper, ) < 0) \
 ret = -1; \
@@ -198,47 +198,47 @@ mymain(void)
 
 DO_TEST("pool-dir", "vol-qcow2",
 NULL, NULL,
-"qcow2-compat", 0, FMT_COMPAT);
+"qcow2-compat", 0);
 DO_TEST("pool-dir", "vol-qcow2-nobacking",
 NULL, NULL,
-"qcow2-nobacking-prealloc-compat", flags, FMT_COMPAT);
+"qcow2-nobacking-prealloc-compat", flags);
 DO_TEST("pool-dir", "vol-qcow2-nobacking",
 "pool-dir", "vol-file",
-"qcow2-nobacking-convert-prealloc-compat", flags, FMT_COMPAT);
+"qcow2-nobacking-convert-prealloc-compat", flags);
 DO_TEST("pool-dir", "vol-qcow2-lazy",
 NULL, NULL,
-"qcow2-lazy", 0, FMT_COMPAT);
+"qcow2-lazy", 0);
 DO_TEST("pool-dir", "vol-qcow2-1.1",
 NULL, NULL,
-"qcow2-1.1", 0, FMT_COMPAT);
+"qcow2-1.1", 0);
 DO_TEST_FAIL("pool-dir", "vol-qcow2-0.10-lazy",
  NULL, NULL,
- "qcow2-0.10-lazy", 0, FMT_COMPAT);
+ "qcow2-0.10-lazy", 0);
 DO_TEST("pool-dir", "vol-qcow2-nobacking",
 "pool-logical", "vol-logical",
-"qcow2-from-logical-compat", 0, FMT_COMPAT);
+"qcow2-from-logical-compat", 0);
 DO_TEST("pool-logical", "vol-logical",
 "pool-dir", "vol-qcow2-nobacking",
-"logical-from-qcow2", 0, FMT_COMPAT);
+"logical-from-qcow2", 0);
 DO_TEST("pool-dir", "vol-qcow2-nocow",
 NULL, NULL,
-"qcow2-nocow-compat", 0, FMT_COMPAT);
+"qcow2-nocow-compat", 0);
 DO_TEST("pool-dir", "vol-qcow2-nocapacity",
 "pool-dir", "vol-file",
-"qcow2-nocapacity-convert-prealloc", flags, FMT_OPTIONS);
+"qcow2-nocapacity-convert-prealloc", flags);
 DO_TEST("pool-dir", "vol-qcow2-zerocapacity",
 NULL, NULL,
-"qcow2-zerocapacity", 0, FMT_COMPAT);
+"qcow2-zerocapacity", 0);
 DO_TEST_FULL(false, VIR_VOL_XML_PARSE_OPT_CAPACITY,
  "pool-dir", "vol-qcow2-nocapacity-backing", NULL, NULL,
- "qcow2-nocapacity", 0, FMT_OPTIONS);
+ "qcow2-nocapacity", 0);
 
 DO_TEST("pool-dir", "vol-file-iso",
 NULL, NULL,
-"iso", 0, FMT_OPTIONS);
+"iso", 0);
 DO_TEST("pool-dir", "vol-file",
 "pool-dir", "vol-file-iso",
-"iso-input", 0, FMT_OPTIONS);
+"iso-input", 0);
 
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-- 
2.13.6

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

[libvirt] [PATCH 1/3] tests: delete most qemu-img test cases assuming FMT_OPTIONS

[Ján Tomko]

We have two leftover "capabilites" for qemu-img:
QEMU_IMG_BACKING_FORMAT_OPTIONS
QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT

The former says we are able to specify the backing format via -o
(which has been the case for a long time now) and the second one
says we can use -o compat to specify the qcow2 version.

Since we require QEMU 1.5.0, we can always assume -o compat,
which was introduced in QEMU 1.1.

Drop the test cases using FMT_OPTIONS which have a FMT_COMPAT
counterpart to prepare for deprecating FMT_OPTIONS (and these flags)
completely.

Signed-off-by: Ján Tomko 
---
 .../qcow2-convert-nobacking.argv   |  2 --
 .../storagevolxml2argvdata/qcow2-from-logical.argv |  2 --
 .../qcow2-nobacking-convert-prealloc.argv  |  2 --
 .../qcow2-nobacking-prealloc.argv  |  2 --
 tests/storagevolxml2argvdata/qcow2.argv|  2 --
 tests/storagevolxml2argvtest.c | 37 --
 6 files changed, 47 deletions(-)
 delete mode 100644 tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv
 delete mode 100644 tests/storagevolxml2argvdata/qcow2-from-logical.argv
 delete mode 100644 
tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv
 delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv
 delete mode 100644 tests/storagevolxml2argvdata/qcow2.argv

diff --git a/tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv 
b/tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv
deleted file mode 100644
index fd1f4c078..0
--- a/tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img convert -f raw -O qcow2 -o encryption=on \
-/var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-from-logical.argv 
b/tests/storagevolxml2argvdata/qcow2-from-logical.argv
deleted file mode 100644
index 6a7581564..0
--- a/tests/storagevolxml2argvdata/qcow2-from-logical.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img convert -f raw -O qcow2 -o encryption=on /dev/HostVG/Swap \
-/var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv 
b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv
deleted file mode 100644
index a49285f89..0
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img convert -f raw -O qcow2 -o encryption=on,preallocation=metadata \
-/var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv 
b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv
deleted file mode 100644
index c74258882..0
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img create -f qcow2 -o encryption=on,preallocation=metadata \
-/var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2.argv 
b/tests/storagevolxml2argvdata/qcow2.argv
deleted file mode 100644
index 6ca9a45f0..0
--- a/tests/storagevolxml2argvdata/qcow2.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img create -f qcow2 -b /dev/null -o backing_fmt=raw,encryption=on \
-/var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 5857d5e35..4353ad467 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -198,40 +198,6 @@ mymain(void)
 
 DO_TEST("pool-dir", "vol-qcow2",
 NULL, NULL,
-"qcow2", 0, FMT_OPTIONS);
-DO_TEST_FAIL("pool-dir", "vol-qcow2",
- NULL, NULL,
- "qcow2-prealloc", flags, FMT_OPTIONS);
-DO_TEST("pool-dir", "vol-qcow2-nobacking",
-NULL, NULL,
-"qcow2-nobacking-prealloc", flags, FMT_OPTIONS);
-DO_TEST("pool-dir", "vol-qcow2-nobacking",
-"pool-dir", "vol-file",
-"qcow2-nobacking-convert-prealloc", flags, FMT_OPTIONS);
-DO_TEST_FAIL("pool-dir", "vol-qcow2",
- "pool-dir", "vol-file",
- "qcow2-convert-nobacking", 0, FMT_OPTIONS);
-DO_TEST_FAIL("pool-dir", "vol-qcow2",
- "pool-dir", "vol-file",
- "qcow2-convert-prealloc", flags, FMT_OPTIONS);
-DO_TEST("pool-dir", "vol-qcow2-lazy",
-NULL, NULL,
-"qcow2-lazy", 0, FMT_OPTIONS);
-DO_TEST("pool-dir", "vol-qcow2-1.1",
-NULL, NULL,
-"qcow2-1.1", 0, FMT_OPTIONS);
-DO_TEST_FAIL("pool-dir", "vol-qcow2-0.10-lazy",
- NULL, NULL,
- "qcow2-0.10-lazy", 0, FMT_OPTIONS);
-DO_TEST("pool-dir", "vol-qcow2-nobacking",
-"pool-logical", "vol-logical",
-"qcow2-from-logical", 0, FMT_OPTIONS);
-DO_TEST("pool-logical", "vol-logical",
-"pool-dir", 

[libvirt] [PATCH 0/3] Ditch qemu-img help scraping

[Ján Tomko]

Ján Tomko (3):
  tests: delete most qemu-img test cases assuming FMT_OPTIONS
  tests: assume FMT_COMPAT for qemu-img tests
  storage: remove qemu-img help scraping

 src/storage/storage_util.c | 73 ++---
 src/storage/storage_util.h |  1 -
 .../qcow2-convert-nobacking.argv   |  2 -
 .../storagevolxml2argvdata/qcow2-from-logical.argv |  2 -
 .../qcow2-nobacking-convert-prealloc.argv  |  2 -
 .../qcow2-nobacking-prealloc.argv  |  2 -
 .../qcow2-nocapacity-convert-prealloc.argv |  2 +-
 tests/storagevolxml2argvdata/qcow2-nocapacity.argv |  2 +-
 tests/storagevolxml2argvdata/qcow2.argv|  2 -
 tests/storagevolxml2argvtest.c | 74 ++
 10 files changed, 24 insertions(+), 138 deletions(-)
 delete mode 100644 tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv
 delete mode 100644 tests/storagevolxml2argvdata/qcow2-from-logical.argv
 delete mode 100644 
tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv
 delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv
 delete mode 100644 tests/storagevolxml2argvdata/qcow2.argv

-- 
2.13.6

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

Re: [libvirt] [PATCH 2/2] storage: Check qemu-img encryption type capability

[Ján Tomko]

On Tue, Apr 17, 2018 at 03:23:33PM -0400, John Ferlan wrote:

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

As of QEMU 2.9, qemu-img has enforced using the "key-secret" for
creation of encrypted volumes. That is, LUKS encryption is now
required and the old (awful) qcow[2] encryption methodolgy is
no longer supported.

In order to check for this, we scan the qemu-img -o help options
looking for "key-secret" and if set, we enforce during the create
volume processing that the about to be encrypted volume doesn't
attempt to use the old crufty encryption mechanism.

Signed-off-by: John Ferlan 
---
src/storage/storage_util.c | 32 ++--
1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 7df52239c2..d2e02a57ca 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -797,6 +797,7 @@ storagePloopResize(virStorageVolDefPtr vol,
enum {
QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
+QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET,
};

static char *
@@ -824,6 +825,14 @@ virStorageBackendQemuImgSupportsCompat(const char *output)
return strstr(output, "\ncompat ");
}

+
+static bool
+virStorageBackendQemuImgRequiresKeySecret(const char *output)
+{
+return strstr(output, "key-secret");
+}
+
+


NACK

adding more -help output scraping just for a nicer error message for a
feature noone should be using in the first place is not worth it.

Jano


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

Re: [libvirt] [PATCH v7 2/9] libxl: pass driver config to libxlMakeDomBuildInfo

[Marek Marczykowski-Górecki]

On Tue, Apr 17, 2018 at 02:00:25PM -0600, Jim Fehlig wrote:
> Your response in the V6 thread about "conflicting types (with
> libxl_get_free_memory_0x040700)" was a good hint. Things work fine for me on
> Xen 4.4 through 4.10 with the following squashed in
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 2c0d1311c..8c4b6c220 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -528,6 +528,7 @@ libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) 
> $(LIBXML_LIBS)
> 
>  virmocklibxl_la_SOURCES = \
> virmocklibxl.c
> +virmocklibxl_la_CFLAGS = $(LIBXL_CFLAGS)
>  virmocklibxl_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
>  virmocklibxl_la_LIBADD = $(MOCKLIBS_LIBS)
> 
> But I'm still confused as to how this works for you. Any idea about that? :-)

Oh, I see it now. For me, -DLIBXL_API_VERSION=0x040400 landed in CFLAGS,
not only LIBXL_CFLAGS. According to config.log it happened just after
testing for xenlight - first, the test using pkg-config failed (something
wrong with my libxl installation), but then fallback to manual check.
And since then, every gcc call have -DLIBXL_API_VERSION=0x040400.

Looking at m4/virt-driver-libxl.m4, I think it's because saved
old_CFLAGS is overridden by LIBVIRT_CHECK_LIB with actual CFLAGS (now
containing -DLIBXL_API_VERSION=0x040400).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


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 2/9] libxl: pass driver config to libxlMakeDomBuildInfo

[Jim Fehlig]

On 04/11/2018 07:03 PM, Marek Marczykowski-Górecki wrote:

Preparation for global nestedhvm configuration - libxlMakeDomBuildInfo
needs access to libxlDriverConfig.
No functional change.

Adjusting tests require slightly more mockup functions, because of
libxlDriverConfigNew() call.

Signed-off-by: Marek Marczykowski-Górecki 
Reviewed-by: Daniel P. Berrangé 
---
Changes since v6:
  - tests: add libxl_get_free_memory mock needed on Xen 4.5
Changes since v4:
  - drop now unneeded parameters
Changes since v3:
  - new patch, preparation
---
  src/libxl/libxl_conf.c | 13 +++--
  src/libxl/libxl_conf.h |  4 +---
  src/libxl/libxl_domain.c   |  2 +-
  tests/libxlxml2domconfigtest.c | 23 ---
  tests/virmocklibxl.c   | 31 +++
  5 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index ae369bc..2565f64 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -271,11 +271,12 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
  
  static int

  libxlMakeDomBuildInfo(virDomainDefPtr def,
-  libxl_ctx *ctx,
+  libxlDriverConfigPtr cfg,
virCapsPtr caps,
libxl_domain_config *d_config)
  {
  virDomainClockDef clock = def->clock;
+libxl_ctx *ctx = cfg->ctx;
  libxl_domain_build_info *b_info = _config->b_info;
  int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
  size_t i;
@@ -2346,17 +2347,17 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, 
virNodeInfoPtr info)
  int
  libxlBuildDomainConfig(virPortAllocatorRangePtr graphicsports,
 virDomainDefPtr def,
-   const char *channelDir LIBXL_ATTR_UNUSED,
-   libxl_ctx *ctx,
-   virCapsPtr caps,
+   libxlDriverConfigPtr cfg,
 libxl_domain_config *d_config)
  {
+virCapsPtr caps = cfg->caps;
+libxl_ctx *ctx = cfg->ctx;
  libxl_domain_config_init(d_config);
  
  if (libxlMakeDomCreateInfo(ctx, def, _config->c_info) < 0)

  return -1;
  
-if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0)

+if (libxlMakeDomBuildInfo(def, cfg, caps, d_config) < 0)
  return -1;
  
  #ifdef LIBXL_HAVE_VNUMA

@@ -2388,7 +2389,7 @@ libxlBuildDomainConfig(virPortAllocatorRangePtr 
graphicsports,
  #endif
  
  #ifdef LIBXL_HAVE_DEVICE_CHANNEL

-if (libxlMakeChannelList(channelDir, def, d_config) < 0)
+if (libxlMakeChannelList(cfg->channelDir, def, d_config) < 0)
  return -1;
  #endif
  
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h

index 0e85dff..633ebf5 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -215,9 +215,7 @@ libxlCreateXMLConf(void);
  int
  libxlBuildDomainConfig(virPortAllocatorRangePtr graphicsports,
 virDomainDefPtr def,
-   const char *channelDir LIBXL_ATTR_UNUSED,
-   libxl_ctx *ctx,
-   virCapsPtr caps,
+   libxlDriverConfigPtr cfg,
 libxl_domain_config *d_config);
  
  static inline void

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ef9a902..0614589 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1261,7 +1261,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
  goto cleanup_dom;
  
  if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def,

-   cfg->channelDir, cfg->ctx, cfg->caps, _config) 
< 0)
+   cfg, _config) < 0)
  goto cleanup_dom;
  
  if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, _config) < 0)

diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index 6eec4c7..9d280e9 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -56,8 +56,8 @@ testCompareXMLToDomConfig(const char *xmlfile,
  int ret = -1;
  libxl_domain_config actualconfig;
  libxl_domain_config expectconfig;
+libxlDriverConfigPtr cfg;
  xentoollog_logger *log = NULL;
-libxl_ctx *ctx = NULL;
  virPortAllocatorRangePtr gports = NULL;
  virDomainXMLOptionPtr xmlopt = NULL;
  virDomainDefPtr vmdef = NULL;
@@ -68,10 +68,18 @@ testCompareXMLToDomConfig(const char *xmlfile,
  libxl_domain_config_init();
  libxl_domain_config_init();
  
+if (!(cfg = libxlDriverConfigNew()))

+goto cleanup;
+
+cfg->caps = caps;
+
  if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, 
XTL_DEBUG, 0)))
  goto cleanup;
  
-if (libxl_ctx_alloc(, LIBXL_VERSION, 0, log) < 0)

+/* replace logger with stderr one */
+libxl_ctx_free(cfg->ctx);
+
+if (libxl_ctx_alloc(>ctx, 

[libvirt] [PATCH 1/2] storage: Separate out the qemu-img help output generation

[John Ferlan]

Separate out and return the output string for future comparison.
Going to need to add new checks shortly.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 897dfdaaee..7df52239c2 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -799,11 +799,10 @@ enum {
 QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
 };
 
-static bool
-virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
+static char *
+virStorageBackendQemuImgCreateHelp(const char *qemuimg)
 {
-bool ret = false;
-char *output;
+char *output = NULL;
 virCommandPtr cmd = NULL;
 
 cmd = virCommandNewArgList(qemuimg, "create", "-o", "?", "-f", "qcow2",
@@ -812,34 +811,40 @@ virStorageBackendQemuImgSupportsCompat(const char 
*qemuimg)
 virCommandAddEnvString(cmd, "LC_ALL=C");
 virCommandSetOutputBuffer(cmd, );
 
-if (virCommandRun(cmd, NULL) < 0)
-goto cleanup;
-
-if (strstr(output, "\ncompat "))
-ret = true;
+ignore_value(virCommandRun(cmd, NULL));
 
- cleanup:
 virCommandFree(cmd);
-VIR_FREE(output);
-return ret;
+return output;
 }
 
 
+static bool
+virStorageBackendQemuImgSupportsCompat(const char *output)
+{
+return strstr(output, "\ncompat ");
+}
+
 static int
 virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
 {
+char *output = NULL;
 /* As of QEMU 0.11 the [-o options] support was added via qemu
  * commit id '9ea2ea71', so we start with that base and figure
  * out what else we have */
 int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
 
+if (!(output = virStorageBackendQemuImgCreateHelp(qemuimg)))
+goto cleanup;
+
 /* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
  * understands. Since we still support QEMU 0.12 and newer, we need
  * to be able to handle the previous format as can be set via a
  * compat=0.10 option. */
-if (virStorageBackendQemuImgSupportsCompat(qemuimg))
+if (virStorageBackendQemuImgSupportsCompat(output))
 ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
 
+ cleanup:
+VIR_FREE(output);
 return ret;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 2/2] storage: Check qemu-img encryption type capability

[John Ferlan]

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

As of QEMU 2.9, qemu-img has enforced using the "key-secret" for
creation of encrypted volumes. That is, LUKS encryption is now
required and the old (awful) qcow[2] encryption methodolgy is
no longer supported.

In order to check for this, we scan the qemu-img -o help options
looking for "key-secret" and if set, we enforce during the create
volume processing that the about to be encrypted volume doesn't
attempt to use the old crufty encryption mechanism.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 7df52239c2..d2e02a57ca 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -797,6 +797,7 @@ storagePloopResize(virStorageVolDefPtr vol,
 enum {
 QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
 QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
+QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET,
 };
 
 static char *
@@ -824,6 +825,14 @@ virStorageBackendQemuImgSupportsCompat(const char *output)
 return strstr(output, "\ncompat ");
 }
 
+
+static bool
+virStorageBackendQemuImgRequiresKeySecret(const char *output)
+{
+return strstr(output, "key-secret");
+}
+
+
 static int
 virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
 {
@@ -843,6 +852,11 @@ virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
 if (virStorageBackendQemuImgSupportsCompat(output))
 ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
 
+/* QEMU 2.9 enforced that qemu-img creation of an encrypted volume
+ * uses LUKS encryption. */
+if (virStorageBackendQemuImgRequiresKeySecret(output))
+ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET;
+
  cleanup:
 VIR_FREE(output);
 return ret;
@@ -934,6 +948,7 @@ 
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
 
 /* storageBackendCreateQemuImgCheckEncryption:
  * @format: format of file found
+ * @imgformat: image format capability
  * @conn: pointer to connection
  * @vol: pointer to volume def
  *
@@ -943,6 +958,7 @@ 
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
  */
 static int
 storageBackendCreateQemuImgCheckEncryption(int format,
+   int imgformat,
const char *type,
virStorageVolDefPtr vol)
 {
@@ -956,6 +972,12 @@ storageBackendCreateQemuImgCheckEncryption(int format,
vol->target.encryption->format);
 return -1;
 }
+if (imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS_KEY_SECRET) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("qemu-img no longer supports qcow encryption, "
+ "use LUKS encryption instead"));
+return -1;
+}
 if (enc->nsecrets > 1) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("too many secrets for qcow encryption"));
@@ -1264,8 +1286,8 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 return NULL;
 
 if (info.encryption &&
-storageBackendCreateQemuImgCheckEncryption(info.format, type,
-   vol) < 0)
+storageBackendCreateQemuImgCheckEncryption(info.format, imgformat,
+   type, vol) < 0)
 return NULL;
 
 
@@ -2359,6 +2381,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
 {
 int ret = -1;
 char *img_tool = NULL;
+int imgformat;
 virCommandPtr cmd = NULL;
 const char *type;
 char *secretPath = NULL;
@@ -2371,6 +2394,10 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
 return -1;
 }
 
+imgformat = virStorageBackendQEMUImgBackingFormat("qemu-img");
+if (imgformat < 0)
+goto cleanup;
+
 if (vol->target.encryption) {
 if (vol->target.format == VIR_STORAGE_FILE_RAW)
 type = "luks";
@@ -2380,6 +2407,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
 storageBackendLoadDefaultSecrets(vol);
 
 if (storageBackendCreateQemuImgCheckEncryption(vol->target.format,
+   imgformat,
type, vol) < 0)
 goto cleanup;
 
-- 
2.13.6

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

[libvirt] [PATCH 0/2] Disallow qemu-img creation of qcow[2] encryption type

[John Ferlan]

Details in the patches.

John Ferlan (2):
  storage: Separate out the qemu-img help output generation
  storage: Check qemu-img encryption type capability

 src/storage/storage_util.c | 63 +++---
 1 file changed, 48 insertions(+), 15 deletions(-)

-- 
2.13.6

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

Re: [libvirt] [PATCH v4 0/5] qemu: add virQEMUBuildBufferEscapeComma in qemu_command.c

[Cole Robinson]

On 04/17/2018 01:33 PM, John Ferlan wrote:
> 
> 
> On 04/16/2018 06:56 PM, Sukrit Bhatnagar wrote:
>> Changes in v4:
>> Changes made in v2 anbd v3 to qemu_command.c are discarded.
>> Some changes introduced in v2 are used to create new smaller patches.
>> virQEMUBuildBufferEscapeComma was applied to:
>> - info->romfile in qemuBuildRomStr
>> - disk->vendor and disk->product in qemuBuildDriveDevStr
>> - fs->src->path in qemuBuildFSStr
>> - fs->dst in qemuBuildFSDevStr
>> - cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
>> - loader->path and loader->nvram in qemuBuildDomainLoaderCommandLine
>>
>>
>> Changes in v3:
>> virQEMUBuildBufferEscapeComma was applied to:
>> - src->hosts->socket in qemuBuildNetworkDriveURI
>> - src->path, src->configFile in qemuBuildNetworkDriveStr
>> - disk->blkdeviotune.group_name in qemuBuildDiskThrottling
>> - net->data.socket.address, net->data.socket.localaddr in
>>   qemuBuildHostNetStr
>> - dev->data.file.path in qemuBuildChrChardevStr
>> - graphics->data.spice.rendernode in
>>   qemuBuildGraphicsSPICECommandLine
>> - smartcard->data.cert.file[i], smartcard->data.cert.database in
>>   qemuBuildSmartcardCommandLine
>>
>> Link to v3: 
>> https://www.redhat.com/archives/libvir-list/2018-April/msg00053.html
>>
>> Changes in v2:
>> virQEMUBuildBufferEscapeComma was applied to:
>> - info->romfile in qemuBuildRomStr
>> - disk->vendor, disk->product in qemuBuildDriveDevStr
>> - fs->src->path in qemuBuildFSStr
>> - fs->dst in qemuBuildFSDevStr
>> - connect= in qemuBuildHostNetStr
>> - fileval handling in qemuBuildChrChardevStr
>> - TYPE_DEV, TYPE_PIPE handling in qemuBuildChrChardevStr
>> - cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
>> - cfg->spiceTLSx509certdir in qemuBuildGraphicsSPICECommandLine
>> - loader->path, loader->nvram usage in
>>   qemuBuildDomainLoaderCommandLine
>>
>> Link to v2: 
>> https://www.redhat.com/archives/libvir-list/2018-March/msg00965.html
>>
>>
>> Sukrit Bhatnagar (5):
>>   qemu: Escape commas for qemuBuildRomStr
>>   qemu: Escape commas for qemuBuildDriveDevStr
>>   qemu: Escape commas for qemuBuildFSStr and qemuBuildFSDevStr
>>   qemu: Escape commas for qemuBuildGraphicsVNCCommandLine
>>   qemu: Escape commas for qemuBuildDomainLoaderCommandLine
>>
>>  src/qemu/qemu_command.c | 47 +--
>>  1 file changed, 29 insertions(+), 18 deletions(-)
>>
> 
> Reviewed-by: John Ferlan 
> (series) and pushed.
> 
> Congrats on your first libvirt patches,
> 
> John

Since I'm guessing these patches were motivated by the BiteSizedTasks
entry, I updated the list there:

https://wiki.libvirt.org/page/BiteSizedTasks#qemu:_Use_comma_escaping_for_more_command_line_values

Sukrit if you hit issues with any of the other entries please let me
know, maybe some of them aren't adequately described

> 
> BTW: All your patches appeared as would a reply-to in the series, I
> think that may be because of not using --no-chain-reply-to on the git
> send-email or git format-patch command line.
> 

In my ~/.gitconfig I have:

[sendemail]
chainreplyto = false


Thanks,
Cole

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

[libvirt] [PATCH 1/3] conf: format/parse as tristate

[Cole Robinson]

 is a bare boolean XML property. We don't really
use this format anymore and instead prefer tristate 
since it's required for modeling on/off/default. If for example future
qemu started enabling vmcoreinfo by default we wouldn't have any way
for the user to turn this off.

Convert it to tristate. For writing XML this is semanticly the same,
 is processed as .

For apps reading guest XML this is technically an API change,
as they might misinterpret , however this
has only been present in libvirt since 3.10.0 and I don't think any
apps are dependent on this yet

Signed-off-by: Cole Robinson 
---
 src/conf/domain_conf.c  | 4 ++--
 tests/qemuxml2xmloutdata/vmcoreinfo.xml | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4dad8e3b2..648057ad4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19281,7 +19281,6 @@ virDomainDefParseXML(xmlDocPtr xml,
 case VIR_DOMAIN_FEATURE_VIRIDIAN:
 case VIR_DOMAIN_FEATURE_PRIVNET:
 case VIR_DOMAIN_FEATURE_HYPERV:
-case VIR_DOMAIN_FEATURE_VMCOREINFO:
 case VIR_DOMAIN_FEATURE_KVM:
 def->features[val] = VIR_TRISTATE_SWITCH_ON;
 break;
@@ -19300,6 +19299,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_VMCOREINFO:
 case VIR_DOMAIN_FEATURE_HAP:
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
@@ -26870,7 +26870,6 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 case VIR_DOMAIN_FEATURE_ACPI:
 case VIR_DOMAIN_FEATURE_PAE:
 case VIR_DOMAIN_FEATURE_VIRIDIAN:
-case VIR_DOMAIN_FEATURE_VMCOREINFO:
 case VIR_DOMAIN_FEATURE_PRIVNET:
 switch ((virTristateSwitch) def->features[i]) {
 case VIR_TRISTATE_SWITCH_ABSENT:
@@ -26891,6 +26890,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 
 break;
 
+case VIR_DOMAIN_FEATURE_VMCOREINFO:
 case VIR_DOMAIN_FEATURE_HAP:
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
diff --git a/tests/qemuxml2xmloutdata/vmcoreinfo.xml 
b/tests/qemuxml2xmloutdata/vmcoreinfo.xml
index d0cd2f2ce..48b75d7d4 100644
--- a/tests/qemuxml2xmloutdata/vmcoreinfo.xml
+++ b/tests/qemuxml2xmloutdata/vmcoreinfo.xml
@@ -9,7 +9,7 @@
 
   
   
-
+
   
   
   destroy
-- 
2.17.0

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

[libvirt] [PATCH 2/3] conf: Add a comment warning about boolean feature XML

[Cole Robinson]

This is the old style and we really shouldn't be adding any more
examples like this. Add a comment to warn devs away

Signed-off-by: Cole Robinson 
---
 docs/schemas/domaincommon.rng | 6 +-
 src/conf/domain_conf.c| 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4cab55f05..015c5b737 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5036,7 +5036,11 @@
 
   
 
-  
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 648057ad4..e303c503e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26871,6 +26871,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 case VIR_DOMAIN_FEATURE_PAE:
 case VIR_DOMAIN_FEATURE_VIRIDIAN:
 case VIR_DOMAIN_FEATURE_PRIVNET:
+/* NOTE: This is for old style  booleans. New XML
+ * should use the explicit state=on|off output below */
 switch ((virTristateSwitch) def->features[i]) {
 case VIR_TRISTATE_SWITCH_ABSENT:
 break;
-- 
2.17.0

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

[libvirt] [PATCH 3/3] domain_capabilities: Report support

[Cole Robinson]

Report domaincaps  if the guest
config accepts 

Signed-off-by: Cole Robinson 
---
This bucks the domaincapabilities trend of always having a child
enum if supported='yes'. Following that trend we would give us
this XML when vmcoreinfo is supported:

  

  on
  off

  

Which is verbose but fine. But it's unclear what we do in the
case when vmcoreinfo isn't supported... do we do:

  

which may not be entirely accurate because the code will still accept
. Or do we do:

  

  off

  

Which is weird IMO. I'm not certain what the semantics of
'supported' are meant to be so I went with this minimal option
but I'm not tied to it

 docs/formatdomaincaps.html.in   | 5 +
 docs/schemas/domaincaps.rng | 7 +++
 src/conf/domain_capabilities.c  | 2 ++
 src/conf/domain_capabilities.h  | 1 +
 src/qemu/qemu_capabilities.c| 3 +++
 tests/domaincapsschemadata/basic.xml| 1 +
 tests/domaincapsschemadata/bhyve_basic.x86_64.xml   | 1 +
 tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml| 1 +
 tests/domaincapsschemadata/bhyve_uefi.x86_64.xml| 1 +
 tests/domaincapsschemadata/full.xml | 1 +
 tests/domaincapsschemadata/libxl-xenfv-usb.xml  | 1 +
 tests/domaincapsschemadata/libxl-xenfv.xml  | 1 +
 tests/domaincapsschemadata/libxl-xenpv-usb.xml  | 1 +
 tests/domaincapsschemadata/libxl-xenpv.xml  | 1 +
 tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml | 1 +
 tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.12.0.s390x.xml| 1 +
 tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml   | 1 +
 tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml  | 1 +
 tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml   | 1 +
 tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml | 1 +
 tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 +
 tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 +
 tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml| 1 +
 30 files changed, 43 insertions(+)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 6bfcaf61c..acdc1cf8a 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -417,6 +417,7 @@
 value3/value
   /enum
 /gic
+vmcoreinfo supported='yes'/
   /features
 /domainCapabilities
 
@@ -441,5 +442,9 @@
   gic element.
 
 
+vmcoreinfo
+
+Reports whether the vmcoreinfo feature can be enabled
+
   
 
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 39053181e..bace0e44a 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -173,6 +173,7 @@
 
   
 
+
   
 
   
@@ -184,6 +185,12 @@
 
   
 
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index f7d9be50f..ccdccd695 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -587,6 +587,8 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
 virBufferAdjustIndent(, 2);
 
 virDomainCapsFeatureGICFormat(, >gic);
+virBufferAsprintf(, "\n",
+  caps->vmcoreinfo ? "yes" : "no");
 
 virBufferAdjustIndent(, -2);
 virBufferAddLit(, "\n");
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index fa4c1e442..5bb028f63 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -157,6 +157,7 @@ struct _virDomainCaps {
 /* add new domain devices here */
 
 virDomainCapsFeatureGIC gic;
+bool vmcoreinfo;
 /* add new domain features here */
 };
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f379fc6d2..0543c0194 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4830,6 +4830,9 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
 domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
 }
 
+domCaps->vmcoreinfo = virQEMUCapsGet(qemuCaps,
+ QEMU_CAPS_DEVICE_VMCOREINFO);
+
 if (virQEMUCapsFillDomainOSCaps(os, firmwares, nfirmwares) < 0 ||
 virQEMUCapsFillDomainCPUCaps(caps, qemuCaps, domCaps) < 0 ||
 virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps,
diff --git a/tests/domaincapsschemadata/basic.xml 
b/tests/domaincapsschemadata/basic.xml
index 

[libvirt] [PATCH 0/3] conf: tweaks

[Cole Robinson]

Patch #1 changes the vmcoreinfo XML schema slightly to be more future
proof, this is technically an API break but I'm not sure it matters,
see the patch for more details.

Patch #3 adds vmcoreinfo reporting to domain capabilities. The
schema doesn't follow the typical pattern so if anyone has other ideas
please comment

Cole Robinson (3):
  conf: format/parse  as tristate
  conf: Add a comment warning about boolean feature XML
  domain_capabilities: Report  support

 docs/formatdomaincaps.html.in   | 5 +
 docs/schemas/domaincaps.rng | 7 +++
 docs/schemas/domaincommon.rng   | 6 +-
 src/conf/domain_capabilities.c  | 2 ++
 src/conf/domain_capabilities.h  | 1 +
 src/conf/domain_conf.c  | 6 --
 src/qemu/qemu_capabilities.c| 3 +++
 tests/domaincapsschemadata/basic.xml| 1 +
 tests/domaincapsschemadata/bhyve_basic.x86_64.xml   | 1 +
 tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml| 1 +
 tests/domaincapsschemadata/bhyve_uefi.x86_64.xml| 1 +
 tests/domaincapsschemadata/full.xml | 1 +
 tests/domaincapsschemadata/libxl-xenfv-usb.xml  | 1 +
 tests/domaincapsschemadata/libxl-xenfv.xml  | 1 +
 tests/domaincapsschemadata/libxl-xenpv-usb.xml  | 1 +
 tests/domaincapsschemadata/libxl-xenpv.xml  | 1 +
 tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml | 1 +
 tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.12.0.s390x.xml| 1 +
 tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml   | 1 +
 tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml  | 1 +
 tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml   | 1 +
 tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml | 1 +
 tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 +
 tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 +
 tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml| 1 +
 tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml| 1 +
 tests/qemuxml2xmloutdata/vmcoreinfo.xml | 2 +-
 33 files changed, 53 insertions(+), 4 deletions(-)

-- 
2.17.0

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

Re: [libvirt] [dbus PATCH 10/25] Implement BlockRebase method for Domain Interface

[Pavel Hrdina]

On Tue, Apr 17, 2018 at 07:26:15PM +0200, Pavel Hrdina wrote:
> On Tue, Apr 17, 2018 at 02:04:29PM +0200, Katerina Koukiou wrote:
> > Signed-off-by: Katerina Koukiou 
> > ---
> >  data/org.libvirt.Domain.xml |  8 
> >  src/domain.c| 29 +
> >  2 files changed, 37 insertions(+)
> > 
> > diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> > index 46a6632..f5c7fb0 100644
> > --- a/data/org.libvirt.Domain.xml
> > +++ b/data/org.libvirt.Domain.xml
> > @@ -91,6 +91,14 @@
> >
> >
> >  
> > +
> > +   > +value="See 
> > https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockRebase"/>

One more thing, we should document that empty string is same as
NULL, for example:

"Empty string can be used to pass a NULL as @base argument."

Pavel


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 0/5] qemu: add virQEMUBuildBufferEscapeComma in qemu_command.c

[John Ferlan]

On 04/16/2018 06:56 PM, Sukrit Bhatnagar wrote:
> Changes in v4:
> Changes made in v2 anbd v3 to qemu_command.c are discarded.
> Some changes introduced in v2 are used to create new smaller patches.
> virQEMUBuildBufferEscapeComma was applied to:
> - info->romfile in qemuBuildRomStr
> - disk->vendor and disk->product in qemuBuildDriveDevStr
> - fs->src->path in qemuBuildFSStr
> - fs->dst in qemuBuildFSDevStr
> - cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
> - loader->path and loader->nvram in qemuBuildDomainLoaderCommandLine
> 
> 
> Changes in v3:
> virQEMUBuildBufferEscapeComma was applied to:
> - src->hosts->socket in qemuBuildNetworkDriveURI
> - src->path, src->configFile in qemuBuildNetworkDriveStr
> - disk->blkdeviotune.group_name in qemuBuildDiskThrottling
> - net->data.socket.address, net->data.socket.localaddr in
>   qemuBuildHostNetStr
> - dev->data.file.path in qemuBuildChrChardevStr
> - graphics->data.spice.rendernode in
>   qemuBuildGraphicsSPICECommandLine
> - smartcard->data.cert.file[i], smartcard->data.cert.database in
>   qemuBuildSmartcardCommandLine
> 
> Link to v3: 
> https://www.redhat.com/archives/libvir-list/2018-April/msg00053.html
> 
> Changes in v2:
> virQEMUBuildBufferEscapeComma was applied to:
> - info->romfile in qemuBuildRomStr
> - disk->vendor, disk->product in qemuBuildDriveDevStr
> - fs->src->path in qemuBuildFSStr
> - fs->dst in qemuBuildFSDevStr
> - connect= in qemuBuildHostNetStr
> - fileval handling in qemuBuildChrChardevStr
> - TYPE_DEV, TYPE_PIPE handling in qemuBuildChrChardevStr
> - cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
> - cfg->spiceTLSx509certdir in qemuBuildGraphicsSPICECommandLine
> - loader->path, loader->nvram usage in
>   qemuBuildDomainLoaderCommandLine
> 
> Link to v2: 
> https://www.redhat.com/archives/libvir-list/2018-March/msg00965.html
> 
> 
> Sukrit Bhatnagar (5):
>   qemu: Escape commas for qemuBuildRomStr
>   qemu: Escape commas for qemuBuildDriveDevStr
>   qemu: Escape commas for qemuBuildFSStr and qemuBuildFSDevStr
>   qemu: Escape commas for qemuBuildGraphicsVNCCommandLine
>   qemu: Escape commas for qemuBuildDomainLoaderCommandLine
> 
>  src/qemu/qemu_command.c | 47 +--
>  1 file changed, 29 insertions(+), 18 deletions(-)
> 

Reviewed-by: John Ferlan 
(series) and pushed.

Congrats on your first libvirt patches,

John

BTW: All your patches appeared as would a reply-to in the series, I
think that may be because of not using --no-chain-reply-to on the git
send-email or git format-patch command line.

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

Re: [libvirt] [dbus PATCH 10/25] Implement BlockRebase method for Domain Interface

[Pavel Hrdina]

On Tue, Apr 17, 2018 at 02:04:29PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  8 
>  src/domain.c| 29 +
>  2 files changed, 37 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index 46a6632..f5c7fb0 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -91,6 +91,14 @@
>
>
>  
> +
> +   +value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockRebase"/>
> +  
> +  
> +  
> +  
> +
>  
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateWithFlags"/>
> diff --git a/src/domain.c b/src/domain.c
> index af8cc73..107355e 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -521,6 +521,34 @@ virtDBusDomainBlockPull(GVariant *inArgs,
>  virtDBusUtilSetLastVirtError(error);
>  }
>  
> +static void
> +virtDBusDomainBlockRebase(GVariant *inArgs,
> +  GUnixFDList *inFDs G_GNUC_UNUSED,
> +  const gchar *objectPath,
> +  gpointer userData,
> +  GVariant **outArgs G_GNUC_UNUSED,
> +  GUnixFDList **outFDs G_GNUC_UNUSED,
> +  GError **error)
> +{
> +virtDBusConnect *connect = userData;
> +g_autoptr(virDomain) domain = NULL;
> +const gchar *disk;
> +const gchar *base;
> +gulong bandwidth;
> +guint flags;
> +
> +g_variant_get(inArgs, "()", , , , );
> +if (g_strcmp0(base, "") == 0)

You can use g_str_equal().

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

Re: [libvirt] [dbus PATCH 08/25] Implement BlockJobAbort method for Domain Interface

[Pavel Hrdina]

On Tue, Apr 17, 2018 at 02:04:27PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  6 ++
>  src/domain.c| 25 +
>  2 files changed, 31 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index 3d8e7b1..480499c 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -69,6 +69,12 @@
>
>
>  
> +
> +   +value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockJobAbort"/>
> +  
> +  
> +
>  
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateWithFlags"/>
> diff --git a/src/domain.c b/src/domain.c
> index f482bb9..fee4a2c 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -434,6 +434,30 @@ virtDBusDomainBlockCommit(GVariant *inArgs,
>  virtDBusUtilSetLastVirtError(error);
>  }
>  
> +static void
> +virtDBusDomainBlockJobAbort(GVariant *inArgs G_GNUC_UNUSED,

inArgs is used so remove G_GNUC_UNUSED.

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

Re: [libvirt] [dbus PATCH 05/25] Implement MemoryPeek method for Domain Interface

[Pavel Hrdina]

On Tue, Apr 17, 2018 at 02:04:24PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  8 
>  src/domain.c| 38 ++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index 85e2cf6..bd30ad4 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -148,6 +148,14 @@
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainManagedSaveRemove"/>
>
>  
> +
> +   +value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMemoryPeek"/>
> +  
> +  

Same as for the previous one, "t" type and gsize in the code.

> +  
> +  
> +
>  
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMemoryStats"/>
> diff --git a/src/domain.c b/src/domain.c
> index c02e289..01f120d 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -859,6 +859,43 @@ virtDBusDomainManagedSaveRemove(GVariant *inArgs,
>  virtDBusUtilSetLastVirtError(error);
>  }
>  
> +static void
> +virtDBusDomainMemoryPeek(GVariant *inArgs,
> + GUnixFDList *inFDs G_GNUC_UNUSED,
> + const gchar *objectPath,
> + gpointer userData,
> + GVariant **outArgs,
> + GUnixFDList **outFDs G_GNUC_UNUSED,
> + GError **error)
> +{
> +virtDBusConnect *connect = userData;
> +g_autoptr(virDomain) domain = NULL;
> +gulong offset;
> +guint size;
> +guint flags;
> +g_autofree guchar *buffer = NULL;
> +GVariantBuilder *builder;
> +GVariant *res;
> +
> +g_variant_get(inArgs, "(tuu)", , , );
> +
> +domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
> +if (!domain)
> +return;
> +
> +buffer = g_new0(guchar, size);
> +if (virDomainMemoryPeek(domain, offset, size, buffer, flags) < 0)
> +virtDBusUtilSetLastVirtError(error);

Missing return in case of error.

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

Re: [libvirt] [PATCH v2 0/2] Fix vmware driver for VMware Fusion 10

[John Ferlan]

On 04/14/2018 05:25 AM, Rainer Müller wrote:
> These changes were required to get VMware Fusion working at all.
> I tested with VMware Fusion 10.1.1 on macOS 10.12 Sierra.
> 
> I am not sure whether calling virCapabilitiesInitCaches() makes sense at
> all on macOS. This function looks highly specific to Linux as it relies
> on /sys/devices/system, so it should probably be wrapped with an
> appropriate #ifdef. I do not feel proficient enough with the libvirt
> internals to make this change, though.
> 
> v1 was here:
> https://www.redhat.com/archives/libvir-list/2018-April/msg00087.html
> 
> diff to v1:
> - Fix a possible segfault when vmrun is not found in PATH.
> 
> Rainer Müller (2):
>   vmware: Fix initialization of VMware Fusion
>   vmware: Failures in cache info init are non-fatal
> 
>  src/vmware/vmware_conf.c   |  7 ++-
>  src/vmware/vmware_driver.c | 11 ++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 

Reviewed-by: John Ferlan 
(series)

and pushed.

Thanks,

John

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

Re: [libvirt] [dbus PATCH 04/25] Implement BlockPeek metohd for Domain Interface

[Pavel Hrdina]

On Tue, Apr 17, 2018 at 02:04:23PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  9 +
>  src/domain.c| 39 +++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index 48b8a95..85e2cf6 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -54,6 +54,15 @@
>
>
>  
> +
> +   +value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateWithFlags"/>
> +  
> +  
> +  

I agree with Jano that this should be "t" type and therefore gsize in
the code.

> +  
> +  
> +
>  
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateWithFlags"/>
> diff --git a/src/domain.c b/src/domain.c
> index 9197755..c02e289 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -383,6 +383,44 @@ virtDBusDomainAttachDevice(GVariant *inArgs,
>  virtDBusUtilSetLastVirtError(error);
>  }
>  
> +static void
> +virtDBusDomainBlockPeek(GVariant *inArgs,
> +GUnixFDList *inFDs G_GNUC_UNUSED,
> +const gchar *objectPath,
> +gpointer userData,
> +GVariant **outArgs,
> +GUnixFDList **outFDs G_GNUC_UNUSED,
> +GError **error)
> +{
> +virtDBusConnect *connect = userData;
> +g_autoptr(virDomain) domain = NULL;
> +const gchar *disk;
> +gulong offset;
> +guint size;
> +guint flags;
> +g_autofree guchar *buffer = NULL;
> +GVariantBuilder *builder;
> +GVariant *res;
> +
> +g_variant_get(inArgs, "()", , , , );
> +
> +domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
> +if (!domain)
> +return;
> +
> +buffer = g_new0(guchar, size);
> +if (virDomainBlockPeek(domain, disk, offset, size, buffer, flags) < 0)
> +virtDBusUtilSetLastVirtError(error);

We need to return from the function if error is set.

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

Re: [libvirt] [dbus PATCH 02/25] Implement SendKey method for Domain Interface

[Pavel Hrdina]

On Tue, Apr 17, 2018 at 02:04:21PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  8 
>  src/domain.c| 46 
> +
>  2 files changed, 54 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index 74b0a62..0b23e75 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -178,6 +178,14 @@
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainResume"/>
>  
> +
> +   +value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSendKey"/>
> +  
> +  
> +  
> +  
> +
>  
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetMemoryFlags"/>
> diff --git a/src/domain.c b/src/domain.c
> index 5ec7686..0c8e1ce 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -1002,6 +1002,51 @@ virtDBusDomainResume(GVariant *inArgs G_GNUC_UNUSED,
>  virtDBusUtilSetLastVirtError(error);
>  }
>  
> +static void
> +virtDBusDomainSendKey(GVariant *inArgs,
> +  GUnixFDList *inFDs G_GNUC_UNUSED,
> +  const gchar *objectPath,
> +  gpointer userData,
> +  GVariant **outArgs G_GNUC_UNUSED,
> +  GUnixFDList **outFDs G_GNUC_UNUSED,
> +  GError **error)
> +
> +{
> +virtDBusConnect *connect = userData;
> +g_autoptr(virDomain) domain = NULL;
> +guint codeset;
> +guint holdtime;
> +const guint *keycodes;
> +gsize nkeycodes = 0;
> +gint ret;
> +guint flags;
> +GVariant *v;
> +
> +v = g_variant_get_child_value(inArgs, 0);
> +codeset  = g_variant_get_uint32(v);
> +g_variant_unref(v);
> +
> +v = g_variant_get_child_value(inArgs, 1);
> +holdtime  = g_variant_get_uint32(v);
> +g_variant_unref(v);
> +
> +v = g_variant_get_child_value(inArgs, 2);
> +keycodes = g_variant_get_fixed_array(v, , sizeof(guint));
> +g_variant_unref(v);
> +
> +v = g_variant_get_child_value(inArgs, 3);
> +flags = g_variant_get_uint32(v);
> +g_variant_unref(v);

This can be simplified into:

+g_variant_get(inArgs, "(uu@auu)", , , , );
+
+keycodes = g_variant_get_fixed_array(v, , sizeof(guint));
+g_variant_unref(v);

> +
> +domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
> +if (!domain)
> +return;
> +
> +ret = virDomainSendKey(domain, codeset, holdtime, (guint *)keycodes, 
> nkeycodes, flags);

Long line, it should not be longer than 80 chars.

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

Re: [libvirt] [PATCH v3 5/5] cfg.mk: Introduce syntax-check rule to prefer VIR_CLASS_NEW

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 06:01:06PM +0200, Michal Privoznik wrote:
> Now that we have macro that does some checks lets forbid raw
> usage of virClassNew() in favor of VIR_CLASS_NEW().
> 
> Signed-off-by: Michal Privoznik 
> Reviewed-by: Erik Skultety 
> ---
>  cfg.mk | 8 
>  1 file changed, 8 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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 v3 4/5] virobject: Check if @parent is the first member in class

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 06:01:05PM +0200, Michal Privoznik wrote:
> Our virObject code relies heavily on the fact that the first
> member of the class struct is type of virObject (or some
> derivation of if). Let's check for that.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virobject.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé 


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 v3 2/5] src: Unify virObject member name

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 06:01:03PM +0200, Michal Privoznik wrote:
> Whenever we declare a new object the first member of the struct
> has to be virObject (or any other member of that family). Now, up
> until now we did not care about the name of the struct member.
> But lets unify it so that we can do some checks at compile time
> later.
> 
> The unified name is 'parent'.
> 
> Signed-off-by: Michal Privoznik 
> Reviewed-by: Erik Skultety 
> ---
>  src/datatypes.h   | 28 ++--
>  src/libvirt-admin.c   |  2 +-
>  src/libvirt-domain-snapshot.c |  2 +-
>  src/libvirt-domain.c  |  2 +-
>  src/libvirt-host.c|  2 +-
>  src/libvirt-interface.c   |  2 +-
>  src/libvirt-network.c |  2 +-
>  src/libvirt-nodedev.c |  2 +-
>  src/libvirt-nwfilter.c|  2 +-
>  src/libvirt-secret.c  |  2 +-
>  src/libvirt-storage.c |  4 ++--
>  src/libvirt-stream.c  |  2 +-
>  src/qemu/qemu_capabilities.c  |  2 +-
>  src/rpc/virnetclientprogram.c |  2 +-
>  src/rpc/virnetserverprogram.c |  2 +-
>  src/rpc/virnetserverservice.c |  2 +-
>  src/util/virdnsmasq.c |  2 +-
>  src/util/virfilecache.c   |  2 +-
>  tests/virfilecachetest.c  |  2 +-
>  19 files changed, 33 insertions(+), 33 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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 v3 3/5] virobject: Introduce VIR_CLASS_NEW() macro

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 06:01:04PM +0200, Michal Privoznik wrote:
> So far we are repeating the following lines over and over:
> 
>   if (!(virSomeObjectClass = virClassNew(virClassForObject(),
>  "virSomeObject",
>  sizeof(virSomeObject),
>  virSomeObjectDispose)))
>   return -1;
> 
> While this works, it is impossible to do some checking. Firstly,
> the class name (the 2nd argument) doesn't match the name in the
> code in all cases (the 3rd argument). Secondly, the current style
> is needlessly verbose. This commit turns example into following:
> 
>   if (!(VIR_CLASS_NEW(virSomeObject,
>   virClassForObject)))
>   return -1;
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/access/viraccessmanager.c   |   5 +-
>  src/bhyve/bhyve_conf.c  |   5 +-
>  src/conf/capabilities.c |   5 +-
>  src/conf/domain_capabilities.c  |  11 +--
>  src/conf/domain_conf.c  |  20 +---
>  src/conf/domain_event.c | 166 
> 
>  src/conf/network_event.c|  14 +--
>  src/conf/node_device_event.c|  21 ++--
>  src/conf/object_event.c |  12 +--
>  src/conf/secret_event.c |  21 ++--
>  src/conf/storage_event.c|  21 ++--
>  src/conf/virdomainobjlist.c |   5 +-
>  src/conf/virinterfaceobj.c  |  10 +-
>  src/conf/virnetworkobj.c|  11 +--
>  src/conf/virnodedeviceobj.c |  10 +-
>  src/conf/virsecretobj.c |  10 +-
>  src/conf/virstorageobj.c|  20 +---
>  src/datatypes.c |   5 +-
>  src/interface/interface_backend_netcf.c |   6 +-
>  src/libvirt-admin.c |   5 +-
>  src/libxl/libxl_conf.c  |   5 +-
>  src/libxl/libxl_domain.c|   5 +-
>  src/libxl/libxl_migration.c |   5 +-
>  src/logging/log_handler.c   |   5 +-
>  src/lxc/lxc_conf.c  |   5 +-
>  src/lxc/lxc_monitor.c   |   5 +-
>  src/node_device/node_device_udev.c  |   5 +-
>  src/qemu/qemu_agent.c   |   5 +-
>  src/qemu/qemu_capabilities.c|   5 +-
>  src/qemu/qemu_conf.c|  11 +--
>  src/qemu/qemu_domain.c  |  51 +++---
>  src/qemu/qemu_monitor.c |   5 +-
>  src/rpc/virkeepalive.c  |   5 +-
>  src/rpc/virnetclient.c  |   5 +-
>  src/rpc/virnetclientprogram.c   |   5 +-
>  src/rpc/virnetclientstream.c|   5 +-
>  src/rpc/virnetdaemon.c  |   5 +-
>  src/rpc/virnetlibsshsession.c   |   5 +-
>  src/rpc/virnetsaslcontext.c |  10 +-
>  src/rpc/virnetserver.c  |   5 +-
>  src/rpc/virnetserverclient.c|   5 +-
>  src/rpc/virnetserverprogram.c   |   5 +-
>  src/rpc/virnetserverservice.c   |   5 +-
>  src/rpc/virnetsocket.c  |   5 +-
>  src/rpc/virnetsshsession.c  |   5 +-
>  src/rpc/virnettlscontext.c  |  10 +-
>  src/security/security_manager.c |   5 +-
>  src/util/virclosecallbacks.c|  11 +--
>  src/util/virdnsmasq.c   |   6 +-
>  src/util/virfdstream.c  |   5 +-
>  src/util/virfilecache.c |   5 +-
>  src/util/virhash.c  |  11 +--
>  src/util/virhostdev.c   |   5 +-
>  src/util/viridentity.c  |   5 +-
>  src/util/virmacmap.c|   5 +-
>  src/util/virmdev.c  |   5 +-
>  src/util/virobject.c|  10 +-
>  src/util/virobject.h|   4 +
>  src/util/virpci.c   |   5 +-
>  src/util/virportallocator.c |   5 +-
>  src/util/virresctrl.c   |  10 +-
>  src/util/virscsi.c  |   5 +-
>  src/util/virscsivhost.c |   5 +-
>  src/util/virusb.c   |   5 +-
>  src/vbox/vbox_common.c  |   5 +-
>  src/vz/vz_driver.c  |   5 +-
>  tests/virfilecachetest.c|   5 +-
>  67 files changed, 167 insertions(+), 535 deletions(-)

Reviewed-by: Daniel P. Berrangé 


but i can't claim I looked closely at anything except the virobject.h
change - i'm assuming the compile time validation is doing the right
thing :-)


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 v3 1/5] datatypes: Rename @parent to @parentName in virNodeDevice

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 06:01:02PM +0200, Michal Privoznik wrote:
> In next patches this name will be needed for a different memeber.
> Also, it makes sense to rename the variable because it does not
> contain reference to parent device, just its name.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/virnodedeviceobj.c  | 2 +-
>  src/datatypes.c  | 2 +-
>  src/datatypes.h  | 2 +-
>  src/libvirt-nodedev.c| 6 +++---
>  src/node_device/node_device_driver.c | 4 ++--
>  src/remote/remote_daemon_dispatch.c  | 4 ++--
>  src/remote/remote_protocol.x | 2 +-
>  src/remote_protocol-structs  | 2 +-
>  src/test/test_driver.c   | 6 +++---
>  9 files changed, 15 insertions(+), 15 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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] [jenkins-ci PATCH v3 3/3] projects: Add libvirt-master-build-website job

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 06:20:46PM +0200, Andrea Bolognani wrote:
> This will ensure libvirt maintains the minimum amount of
> compatibility with CentOS 6 that running its website on that
> platform requires.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  guests/host_vars/libvirt-centos-6/main.yml |  3 +++
>  projects/libvirt.yaml  | 11 +++
>  2 files changed, 14 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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] [jenkins-ci PATCH v3 1/3] jobs: Introduce machine groups

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 06:20:44PM +0200, Andrea Bolognani wrote:
> We're running most of the jobs on all machines, with the major
> notable exceptions being the various *-rpm jobs, which of course
> only make sense for RPM-based distributions, and the MinGW jobs,
> which we only run on Fedora Rawhide.
> 
> Instead of listing machines over and over again, define two
> list globally and refer to them by name. Ad-hoc machine lists
> are still needed in a few places, but overall this cuts down on
> repetition significantly.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  jobs/defaults.yaml| 16 
>  projects/libosinfo.yaml   | 16 ++--
>  projects/libvirt-cim.yaml |  6 +-
>  projects/libvirt-dbus.yaml|  6 +-
>  projects/libvirt-glib.yaml| 22 --
>  projects/libvirt-go-xml.yaml  | 10 +-
>  projects/libvirt-go.yaml  | 10 +-
>  projects/libvirt-perl.yaml| 16 ++--
>  projects/libvirt-python.yaml  | 16 ++--
>  projects/libvirt.yaml | 33 +
>  projects/osinfo-db-tools.yaml | 16 ++--
>  projects/osinfo-db.yaml   | 16 ++--
>  projects/virt-viewer.yaml | 22 --
>  13 files changed, 51 insertions(+), 154 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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] [jenkins-ci PATCH 3/3] jobs: Drop autotools-mingw-job

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 05:43:12PM +0200, Andrea Bolognani wrote:
> Now that we have variants and we've removed all uses of custom
> environment variables, we can convert all jobs that use the
> autotools-mingw-job template to the autotools-build-job plus
> a few overrides.
> 
> As a consequence of this, mingw32 and mingw64 builds will be
> split into separate jobs.

Must remember to purge the old jobs from jenkins manually and
delete the workspace...

> 
> Signed-off-by: Andrea Bolognani 
> ---
>  jobs/autotools.yaml| 80 
> --
>  jobs/defaults.yaml | 16 ++
>  projects/libvirt-glib.yaml | 12 ++-
>  projects/libvirt.yaml  | 12 ++-
>  projects/virt-viewer.yaml  | 12 ++-
>  5 files changed, 49 insertions(+), 83 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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] [jenkins-ci PATCH 1/3] jobs: Introduce variants

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 05:43:10PM +0200, Andrea Bolognani wrote:
> This optional feature will allow us to reuse existing job
> templates for things like MinGW or website builds.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  jobs/autotools.yaml| 20 ++--
>  jobs/defaults.yaml |  1 +
>  jobs/generic.yaml  | 16 
>  jobs/go.yaml   |  8 
>  jobs/perl-makemaker.yaml   | 12 ++--
>  jobs/perl-modulebuild.yaml | 12 ++--
>  jobs/python-distutils.yaml | 12 ++--
>  7 files changed, 41 insertions(+), 40 deletions(-)

Reviewed-by: Daniel P. Berrangé 



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] [jenkins-ci PATCH 2/3] jobs: Tweak autotools-mingw-job template

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 05:43:11PM +0200, Andrea Bolognani wrote:
> Make it more similar to the autotools-build-job by dropping the
> custom $PREFIX variable and redefining the standard $VIRT_PREFIX
> instead, which also makes $PKG_CONFIG_PATH shorter, and moving
> all environment variables together.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  jobs/autotools.yaml | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrangé 

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 7/7] qemu: Check for missing 'return' in qemuMonitorJSONCheckReply

[John Ferlan]

If the "return" is not in the reply, then the subsequent
virJSONValueGetType will not be happy.

Found by Coverity

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_monitor_json.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 410fa178b2..681c0575c3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -423,7 +423,12 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
 if (qemuMonitorJSONCheckError(cmd, reply) < 0)
 return -1;
 
-data = virJSONValueObjectGet(reply, "return");
+if (!(data = virJSONValueObjectGet(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing 'return' for returned data"));
+return -1;
+}
+
 if (virJSONValueGetType(data) != type) {
 char *cmdstr = virJSONValueToString(cmd, false);
 char *retstr = virJSONValueToString(data, false);
-- 
2.13.6

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

[libvirt] [PATCH 6/7] qemu: Fix possible memory leak in migration param processing

[John Ferlan]

If virJSONValueArraySize(caps) <= 0, then we will still need to
virJSONValueFree(caps) because qemuMonitorSetMigrationCapabilities
won't consume it.

Found by Coverity

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_migration_params.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 4f3b239637..3bbe50a8ed 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -771,6 +771,7 @@ qemuMigrationParamsApply(virQEMUDriverPtr driver,
 migParams->params[xbzrle].set = true;
 
 virJSONValueFree(params);
+virJSONValueFree(caps);
 
 return ret;
 }
-- 
2.13.6

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

[libvirt] [PATCH 2/7] remote: Fix usage of ATTRIBUTE_FALLTHROUGH

[John Ferlan]

Move to within the #if since the #else portion ends with a goto
and that raised concern by Coverity.

Signed-off-by: John Ferlan 
---
 src/remote/remote_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index d3b588c374..12f7d47388 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -928,6 +928,7 @@ doRemoteOpen(virConnectPtr conn,
 if (!priv->tls)
 goto failed;
 priv->is_secure = 1;
+ATTRIBUTE_FALLTHROUGH;
 #else
 (void)tls_priority;
 (void)sanity;
@@ -937,7 +938,6 @@ doRemoteOpen(virConnectPtr conn,
 goto failed;
 #endif
 
-ATTRIBUTE_FALLTHROUGH;
 case trans_tcp:
 priv->client = virNetClientNewTCP(priv->hostname, port, AF_UNSPEC);
 if (!priv->client)
-- 
2.13.6

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

[libvirt] [PATCH 5/7] tests: Add checks for possible errors

[John Ferlan]

If virJSONValueObjectGetArray fails to find "members" in @localroot,
then using @rootmembers in subsequent calls which assume that it was
successful will not go well. So add a check that it was successfully
fetched and an error if not.

Similarly virJSONValueArraySize returns an 'ssize_t', so the callers
should use the right type and they should check the return value
against -1 and print an error if something is wrong.

Found by Coverity

Signed-off-by: John Ferlan 
---
 tests/testutilsqemuschema.c | 40 +---
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 21f5d119e8..79c526ac7c 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -97,14 +97,19 @@ struct testQEMUSchemaValidateObjectMemberData {
 
 static virJSONValuePtr
 testQEMUSchemaStealObjectMemberByName(const char *name,
-  virJSONValuePtr members)
+  virJSONValuePtr members,
+  virBufferPtr debug)
 {
 virJSONValuePtr member;
 virJSONValuePtr ret = NULL;
-size_t n;
+ssize_t n;
 size_t i;
 
-n = virJSONValueArraySize(members);
+if ((n = virJSONValueArraySize(members)) < 0) {
+virBufferAddLit(debug, "ERROR: members size < 0");
+return NULL;
+}
+
 for (i = 0; i < n; i++) {
 member = virJSONValueArrayGet(members, i);
 
@@ -132,7 +137,7 @@ testQEMUSchemaValidateObjectMember(const char *key,
 virBufferStrcat(data->debug, key, ": ", NULL);
 
 /* lookup 'member' entry for key */
-if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, 
data->rootmembers))) {
+if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, 
data->rootmembers, data->debug))) {
 virBufferAddLit(data->debug, "ERROR: attribute not in schema");
 goto cleanup;
 }
@@ -188,7 +193,7 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
  virHashTablePtr schema,
  virBufferPtr debug)
 {
-size_t n;
+ssize_t n;
 size_t i;
 virJSONValuePtr variants = NULL;
 virJSONValuePtr variant;
@@ -203,7 +208,11 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
 return -2;
 }
 
-n = virJSONValueArraySize(variants);
+if ((n = virJSONValueArraySize(variants)) < 0) {
+virBufferAddLit(debug, "ERROR: 'variants' array size < 0");
+return -2;
+}
+
 for (i = 0; i < n; i++) {
 variant = virJSONValueArrayGet(variants, i);
 
@@ -307,7 +316,10 @@ testQEMUSchemaValidateObject(virJSONValuePtr obj,
 
 
 /* validate members */
-data.rootmembers = virJSONValueObjectGetArray(localroot, "members");
+if (!(data.rootmembers = virJSONValueObjectGetArray(localroot, 
"members"))) {
+virBufferAddLit(debug, "ERROR: missing attribute 'members'\n");
+goto cleanup;
+}
 if (virJSONValueObjectForeachKeyValue(obj,
   testQEMUSchemaValidateObjectMember,
   ) < 0)
@@ -342,7 +354,7 @@ testQEMUSchemaValidateEnum(virJSONValuePtr obj,
 const char *objstr;
 virJSONValuePtr values = NULL;
 virJSONValuePtr value;
-size_t n;
+ssize_t n;
 size_t i;
 
 if (virJSONValueGetType(obj) != VIR_JSON_TYPE_STRING) {
@@ -358,7 +370,10 @@ testQEMUSchemaValidateEnum(virJSONValuePtr obj,
 return -2;
 }
 
-n = virJSONValueArraySize(values);
+if ((n = virJSONValueArraySize(values)) < 0) {
+virBufferAddLit(debug, "ERROR: enum values array size < 0");
+return -2;
+}
 for (i = 0; i < n; i++) {
 value = virJSONValueArrayGet(values, i);
 
@@ -423,7 +438,7 @@ testQEMUSchemaValidateAlternate(virJSONValuePtr obj,
 {
 virJSONValuePtr members;
 virJSONValuePtr member;
-size_t n;
+ssize_t n;
 size_t i;
 const char *membertype;
 virJSONValuePtr memberschema;
@@ -439,7 +454,10 @@ testQEMUSchemaValidateAlternate(virJSONValuePtr obj,
 virBufferAdjustIndent(debug, 3);
 indent = virBufferGetIndent(debug, false);
 
-n = virJSONValueArraySize(members);
+if ((n = virJSONValueArraySize(members)) < 0) {
+virBufferAddLit(debug, "ERROR: alternate schema 'members' array size < 
0");
+return -2;
+}
 for (i = 0; i < n; i++) {
 membertype = NULL;
 
-- 
2.13.6

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

[libvirt] [PATCH 4/7] tests: Don't call virNetServerClientClose without valid client

[John Ferlan]

If @client hasn't been opened, then don't call virNetServerClientClose
since that'll cause certain failure.

Found by Coverity

Signed-off-by: John Ferlan 
---
 tests/virnetserverclienttest.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c
index 398b46928c..aa3f0bcf9b 100644
--- a/tests/virnetserverclienttest.c
+++ b/tests/virnetserverclienttest.c
@@ -152,7 +152,8 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED)
 ret = 0;
  cleanup:
 virObjectUnref(sock);
-virNetServerClientClose(client);
+if (!client)
+virNetServerClientClose(client);
 virObjectUnref(client);
 virObjectUnref(ident);
 VIR_FORCE_CLOSE(sv[0]);
-- 
2.13.6

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

[libvirt] [PATCH 3/7] tests: Return failure if log not fopen'd

[John Ferlan]

If @log is not fopen'd then, going to cleanup and calling fclose
will make for an unhappy caller. So just fail immediately instead
since there's nothing to clean up.

Found by Coverity

Signed-off-by: John Ferlan 
---
 tests/commandhelper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 1da2834aa4..bf91550ede 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -67,7 +67,7 @@ int main(int argc, char **argv) {
 int ret = EXIT_FAILURE;
 
 if (!log)
-goto cleanup;
+return ret;
 
 for (i = 1; i < argc; i++)
 fprintf(log, "ARG:%s\n", argv[i]);
-- 
2.13.6

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

[libvirt] [PATCH 1/7] conf: Add error checking to virDomainSnapshotDiskDefFormat

[John Ferlan]

Commit id '43f2ccdc' called virDomainDiskSourceDefFormatInternal
rather than formatting the the disk source inline. However, it
did not handle the case where the helper failed. Over time the
helper has been renamed to virDomainDiskSourceFormat. Similar to
other consumers, if virDomainDiskSourceFormat fails, then the
formatting could be off, so it's better to fail than to continue
on with some possibly bad data. Alter the function and the caller
to check status and jump to error in that case.

Found by Coverity

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

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index d7b086242b..787c3d0feb 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -662,7 +662,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
 return ret;
 }
 
-static void
+static int
 virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virDomainSnapshotDiskDefPtr disk,
virDomainXMLOptionPtr xmlopt)
@@ -670,7 +670,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
 int type = disk->src->type;
 
 if (!disk->name)
-return;
+return 0;
 
 virBufferEscapeString(buf, "name);
 if (disk->snapshot > 0)
@@ -679,7 +679,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
 
 if (!disk->src->path && disk->src->format == 0) {
 virBufferAddLit(buf, "/>\n");
-return;
+return 0;
 }
 
 virBufferAsprintf(buf, " type='%s'>\n", virStorageTypeToString(type));
@@ -688,10 +688,12 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
 if (disk->src->format > 0)
 virBufferEscapeString(buf, "\n",
   
virStorageFileFormatTypeToString(disk->src->format));
-virDomainDiskSourceFormat(buf, disk->src, 0, 0, xmlopt);
+if (virDomainDiskSourceFormat(buf, disk->src, 0, 0, xmlopt) < 0)
+return -1;
 
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
+return 0;
 }
 
 
@@ -741,8 +743,10 @@ virDomainSnapshotDefFormat(const char *domain_uuid,
 if (def->ndisks) {
 virBufferAddLit(, "\n");
 virBufferAdjustIndent(, 2);
-for (i = 0; i < def->ndisks; i++)
-virDomainSnapshotDiskDefFormat(, >disks[i], xmlopt);
+for (i = 0; i < def->ndisks; i++) {
+if (virDomainSnapshotDiskDefFormat(, >disks[i], xmlopt) < 
0)
+goto error;
+}
 virBufferAdjustIndent(, -2);
 virBufferAddLit(, "\n");
 }
-- 
2.13.6

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

[libvirt] [PATCH 0/7] Addressing some Coverity complaints...

[John Ferlan]

My current pile is 48 patches - most of those are known false
positives or have been posted before and either rejected or
ignored. In any case, the following is a list of more recent
adjustments from my coverity tree. Seems we've largely moved
away from resource leaks and into return value checking issues.
Some good, some not so good. 


John Ferlan (7):
  conf: Add error checking to virDomainSnapshotDiskDefFormat
  remote: Fix usage of ATTRIBUTE_FALLTHROUGH
  tests: Return failure if log not fopen'd
  tests: Don't call virNetServerClientClose without valid client
  tests: Add checks for possible errors
  qemu: Fix possible memory leak in migration param processing
  qemu: Check for missing 'return' in qemuMonitorJSONCheckReply

 src/conf/snapshot_conf.c | 16 ++--
 src/qemu/qemu_migration_params.c |  1 +
 src/qemu/qemu_monitor_json.c |  7 ++-
 src/remote/remote_driver.c   |  2 +-
 tests/commandhelper.c|  2 +-
 tests/testutilsqemuschema.c  | 40 +---
 tests/virnetserverclienttest.c   |  3 ++-
 7 files changed, 50 insertions(+), 21 deletions(-)

-- 
2.13.6

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

[libvirt] [jenkins-ci PATCH v3 3/3] projects: Add libvirt-master-build-website job

[Andrea Bolognani]

This will ensure libvirt maintains the minimum amount of
compatibility with CentOS 6 that running its website on that
platform requires.

Signed-off-by: Andrea Bolognani 
---
 guests/host_vars/libvirt-centos-6/main.yml |  3 +++
 projects/libvirt.yaml  | 11 +++
 2 files changed, 14 insertions(+)

diff --git a/guests/host_vars/libvirt-centos-6/main.yml 
b/guests/host_vars/libvirt-centos-6/main.yml
index 06fd496..fe9a47b 100644
--- a/guests/host_vars/libvirt-centos-6/main.yml
+++ b/guests/host_vars/libvirt-centos-6/main.yml
@@ -1,3 +1,6 @@
 ---
 PERL5LIB: $VIRT_PREFIX/lib64/perl5
 PYTHONPATH: $VIRT_PREFIX/lib64/python2.6/site-packages
+
+projects:
+  - libvirt+website
diff --git a/projects/libvirt.yaml b/projects/libvirt.yaml
index 56b82e9..b720757 100644
--- a/projects/libvirt.yaml
+++ b/projects/libvirt.yaml
@@ -43,3 +43,14 @@
   local_env: '{mingw64_local_env}'
   autogen_args: '{mingw64_autogen_args}'
   machines: '{mingw_machines}'
+  - generic-build-job:
+  parent_jobs:
+  variant: -website
+  command: |
+mkdir build
+cd build
+../autogen.sh --without-libvirtd --without-macvtap
+$MAKE -j{smp} -C docs/
+$MAKE -j{smp} dist
+  machines:
+- libvirt-centos-6
-- 
2.14.3

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

[libvirt] [jenkins-ci PATCH v3 2/3] guests: Introduce libvirt+website project

[Andrea Bolognani]

Building the website and creating distribution tarballs
requires very few dependencies compared to performing a full
build of libvirt, so we introduce a separate, leaner project
for the purpose.

Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Andrea Bolognani 
---
Although this patch has been fully ACKed, I haven't pushed it
yet because it doesn't make sense without the next one.

 guests/vars/projects/libvirt+website.yml | 5 +
 1 file changed, 5 insertions(+)
 create mode 100644 guests/vars/projects/libvirt+website.yml

diff --git a/guests/vars/projects/libvirt+website.yml 
b/guests/vars/projects/libvirt+website.yml
new file mode 100644
index 000..128ed8c
--- /dev/null
+++ b/guests/vars/projects/libvirt+website.yml
@@ -0,0 +1,5 @@
+---
+packages:
+  - libxml2
+  - rpcgen
+  - xsltproc
-- 
2.14.3

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

[libvirt] [jenkins-ci PATCH v3 0/3] Mostly drop CentOS 6

[Andrea Bolognani]

This applies on top of

  https://www.redhat.com/archives/libvir-list/2018-April/msg01526.html

Changes from [v2]:

  * several patches have been pushed already;

  * define the libvirt-master-build-website job using the newly
variants-enabled generic-build-job template;

  * add 'mingw_machines' variable along with 'all_machines'
and 'rpm_machines'.

Changes from [v1]:

  * instead of dropping CentOS 6 altogether, simply stop building
pretty much all projects on it;

  * introduce a new job type specifically for whatever little
stuff we need to ensure still works on CentOS 6;

  * the end result is still a nice cleanup thanks to not repeating
the list of machines all over the place; moreover, the
message about our support of the OS is very clear because it
disappears from all but one page on the CI web interface.

[v1] https://www.redhat.com/archives/libvir-list/2018-April/msg00453.html
[v2] https://www.redhat.com/archives/libvir-list/2018-April/msg01051.html

Andrea Bolognani (3):
  jobs: Introduce machine groups
  guests: Introduce libvirt+website project
  projects: Add libvirt-master-build-website job

 guests/host_vars/libvirt-centos-6/main.yml |  3 +++
 guests/vars/projects/libvirt+website.yml   |  5 
 jobs/defaults.yaml | 16 
 projects/libosinfo.yaml| 16 ++--
 projects/libvirt-cim.yaml  |  6 +
 projects/libvirt-dbus.yaml |  6 +
 projects/libvirt-glib.yaml | 22 +++-
 projects/libvirt-go-xml.yaml   | 10 +--
 projects/libvirt-go.yaml   | 10 +--
 projects/libvirt-perl.yaml | 16 ++--
 projects/libvirt-python.yaml   | 16 ++--
 projects/libvirt.yaml  | 42 --
 projects/osinfo-db-tools.yaml  | 16 ++--
 projects/osinfo-db.yaml| 16 ++--
 projects/virt-viewer.yaml  | 22 +++-
 15 files changed, 69 insertions(+), 153 deletions(-)
 create mode 100644 guests/vars/projects/libvirt+website.yml

-- 
2.14.3

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

[libvirt] [jenkins-ci PATCH v3 1/3] jobs: Introduce machine groups

[Andrea Bolognani]

We're running most of the jobs on all machines, with the major
notable exceptions being the various *-rpm jobs, which of course
only make sense for RPM-based distributions, and the MinGW jobs,
which we only run on Fedora Rawhide.

Instead of listing machines over and over again, define two
list globally and refer to them by name. Ad-hoc machine lists
are still needed in a few places, but overall this cuts down on
repetition significantly.

Signed-off-by: Andrea Bolognani 
---
 jobs/defaults.yaml| 16 
 projects/libosinfo.yaml   | 16 ++--
 projects/libvirt-cim.yaml |  6 +-
 projects/libvirt-dbus.yaml|  6 +-
 projects/libvirt-glib.yaml| 22 --
 projects/libvirt-go-xml.yaml  | 10 +-
 projects/libvirt-go.yaml  | 10 +-
 projects/libvirt-perl.yaml| 16 ++--
 projects/libvirt-python.yaml  | 16 ++--
 projects/libvirt.yaml | 33 +
 projects/osinfo-db-tools.yaml | 16 ++--
 projects/osinfo-db.yaml   | 16 ++--
 projects/virt-viewer.yaml | 22 --
 13 files changed, 51 insertions(+), 154 deletions(-)

diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml
index 5527546..99e8b62 100644
--- a/jobs/defaults.yaml
+++ b/jobs/defaults.yaml
@@ -4,6 +4,22 @@
 branch: master
 variant: ''
 node: libvirt
+all_machines:
+  - libvirt-centos-7
+  - libvirt-debian-8
+  - libvirt-debian-9
+  - libvirt-fedora-26
+  - libvirt-fedora-27
+  - libvirt-fedora-rawhide
+  - libvirt-freebsd-10
+  - libvirt-freebsd-11
+rpm_machines:
+  - libvirt-centos-7
+  - libvirt-fedora-26
+  - libvirt-fedora-27
+  - libvirt-fedora-rawhide
+mingw_machines:
+  - libvirt-fedora-rawhide
 global_env: |
 local_env: |
 mingw32_local_env: |
diff --git a/projects/libosinfo.yaml b/projects/libosinfo.yaml
index ac04027..0d25447 100644
--- a/projects/libosinfo.yaml
+++ b/projects/libosinfo.yaml
@@ -1,15 +1,7 @@
 
 - project:
 name: libosinfo
-machines:
-  - libvirt-centos-7
-  - libvirt-debian-8
-  - libvirt-debian-9
-  - libvirt-fedora-26
-  - libvirt-fedora-27
-  - libvirt-fedora-rawhide
-  - libvirt-freebsd-10
-  - libvirt-freebsd-11
+machines: '{all_machines}'
 title: libosinfo
 jobs:
   - autotools-build-job:
@@ -20,8 +12,4 @@
   parent_jobs: 'libosinfo-master-syntax-check'
   - autotools-rpm-job:
   parent_jobs: 'libosinfo-master-check'
-  machines:
-- libvirt-centos-7
-- libvirt-fedora-26
-- libvirt-fedora-27
-- libvirt-fedora-rawhide
+  machines: '{rpm_machines}'
diff --git a/projects/libvirt-cim.yaml b/projects/libvirt-cim.yaml
index 160faaf..dff3976 100644
--- a/projects/libvirt-cim.yaml
+++ b/projects/libvirt-cim.yaml
@@ -1,11 +1,7 @@
 
 - project:
 name: libvirt-cim
-machines:
-  - libvirt-centos-7
-  - libvirt-fedora-26
-  - libvirt-fedora-27
-  - libvirt-fedora-rawhide
+machines: '{rpm_machines}'
 title: libvirt CIM
 jobs:
   - autotools-build-job:
diff --git a/projects/libvirt-dbus.yaml b/projects/libvirt-dbus.yaml
index 7e761c7..52cd60c 100644
--- a/projects/libvirt-dbus.yaml
+++ b/projects/libvirt-dbus.yaml
@@ -24,8 +24,4 @@
 - libvirt-freebsd-11
   - autotools-rpm-job:
   parent_jobs: 'libvirt-dbus-master-check'
-  machines:
-- libvirt-centos-7
-- libvirt-fedora-26
-- libvirt-fedora-27
-- libvirt-fedora-rawhide
+  machines: '{rpm_machines}'
diff --git a/projects/libvirt-glib.yaml b/projects/libvirt-glib.yaml
index c56e5d3..2d0e721 100644
--- a/projects/libvirt-glib.yaml
+++ b/projects/libvirt-glib.yaml
@@ -1,15 +1,7 @@
 
 - project:
 name: libvirt-glib
-machines:
-  - libvirt-centos-7
-  - libvirt-debian-8
-  - libvirt-debian-9
-  - libvirt-fedora-26
-  - libvirt-fedora-27
-  - libvirt-fedora-rawhide
-  - libvirt-freebsd-10
-  - libvirt-freebsd-11
+machines: '{all_machines}'
 title: Libvirt GLib
 jobs:
   - autotools-build-job:
@@ -21,22 +13,16 @@
   parent_jobs: 'libvirt-glib-master-syntax-check'
   - autotools-rpm-job:
   parent_jobs: 'libvirt-glib-master-check'
-  machines:
-- libvirt-centos-7
-- libvirt-fedora-26
-- libvirt-fedora-27
-- libvirt-fedora-rawhide
+  machines: '{rpm_machines}'
   - autotools-build-job:
   parent_jobs:
   variant: -mingw32
   local_env: '{mingw32_local_env}'
   autogen_args: '{mingw32_autogen_args}'
-  machines:
-- libvirt-fedora-rawhide
+  machines: '{mingw_machines}'
   - autotools-build-job:
   parent_jobs:
   

Re: [libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 10:18:58AM +0200, Jiri Denemark wrote:
> On Mon, Apr 16, 2018 at 19:16:05 -0400, Collin Walling wrote:
> > [Overview]
> > 
> > This patch series implements an interface to "query-cpu-model-comparison"
> > (available QEMU ~2.8.0) via virsh cpu-compare.
> > 
> > [Using This Feature]
> > 
> > Run virsh cpu-compare (ensure you are running the virsh in your build dir) 
> > and
> > pass it an xml file describing a cpu definition. You can copy the cpu xml 
> > from
> > virsh capabilities (if you want to compare the host cpu to itself), or a 
> > cpu 
> > defined in any guest xml. Additionally, you can create a cpu xml as such 
> > (e.g.
> > for s390x):
> > 
> > 
> > s390x
> > model_name
> > 
> > 
> > 
> > NOTE: the presence of  is optional and it will treat the cpu defined 
> > in 
> >   the xml as a host cpu. This will disregard all feature policies (i.e. 
> >   all features listed will behave with policy='require', even if 
> > disable 
> >   is specified).
> > 
> > NOTE: as s390x only supports feature policies 'require' and 'disable', I am
> >   uncertain how to handle the other policies when parsing CPUDef to 
> > JSON.
> > 
> > [Example Output]
> > 
> > On an s390x system running a z13.2, this is the expected output (where each 
> > file
> > describes a CPU model corresponding to the name of the file):
> > 
> > $ virsh cpu-compare zEC12.xml
> > Host CPU is a superset of CPU described in zEC12.xml
> > 
> > $ virsh cpu-compare z13.2.xml
> > CPU described in z13.2.xml is identical to host CPU
> > 
> > $ virsh cpu-compare z14.xml
> > CPU described in z14.xml is incompatible with host CPU
> > 
> > $ virsh cpu-compare z14.xml --error
> > error: Failed to compare host CPU with z14.xml
> > error: the CPU is incompatible with host CPU
> > 
> > $ virsh cpu-compare z12345.xml 
> > error: Failed to compare host CPU with z12345cpu.xml
> > error: internal error: unable to execute QEMU command 
> > 'query-cpu-model-comparison': The CPU definition 'z12345-base' is unknown.
> > 
> > [Patch Rundown]
> > 
> > The first patch copies the host CPU definition from qemuCaps to virCaps so
> > we can easily access the host CPU model and features when we handle the CPU
> > comparison in qemu_driver. Note that we take care to not clobber anything
> > already stored in the host CPU definition until we have successfully 
> > constructed a new host CPU definition.
> 
> I think this is a wrong approach. You'd be basically giving random
> answers depending on which QEMU binary is probed first. The reason for
> storing the CPU model in qemuCaps is that it is tight to a particular
> QEMU binary rather than the host itself. The model reported by one
> binary may not be usable with other binaries and this applies to any
> comparisons or other operations done with this CPU model.
> 
> In other words, we need to introduce a new set of CPU related APIs which
> will take more arguments so that the caller may specify what binary,
> virt type, and machine type they want to use. In other words, the APIs
> should support parameters similar to virConnectGetDomainCapabilities().

FYI, in addition to all this, the current 'virsh cpu-compare' command
is in fact 100% accurate in what it is reporting today and we can not
change it. The 'virsh cpu-compare' command is answering the question

   "Is this CPU model compatible with the host CPU model"

The problem is that when doing live migration, we don't care about
this question. We were asking the wrong question. We instead need
to ask

   "Is this CPU model compatible with the hypervisor CPU model"

We can't simply change the semantics of the existing API to answer
a different question, because the original question is still relevant
and useful for some usage scenarios. So yes, we must have a new API
that lets us ask the right question for live migration compat.

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 v3 4/5] virobject: Check if @parent is the first member in class

[Michal Privoznik]

Our virObject code relies heavily on the fact that the first
member of the class struct is type of virObject (or some
derivation of if). Let's check for that.

Signed-off-by: Michal Privoznik 
---
 src/util/virobject.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virobject.h b/src/util/virobject.h
index ed1a117b09..77ebad1e8b 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -76,7 +76,8 @@ virClassPtr virClassForObjectRWLockable(void);
 # endif
 
 # define VIR_CLASS_NEW(name, prnt) \
-(name##Class = virClassNew(prnt, #name, sizeof(name), name##Dispose))
+verify_expr(offsetof(name, parent) == 0, \
+  (name##Class = virClassNew(prnt, #name, sizeof(name), name##Dispose)))
 
 virClassPtr
 virClassNew(virClassPtr parent,
-- 
2.16.1

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

[libvirt] [PATCH v3 0/5] virClassNew rework

[Michal Privoznik]

*** Some BLURB HERE. Too tired to write something useful. ***

Michal Privoznik (5):
  datatypes: Rename @parent to @parentName in virNodeDevice
  src: Unify virObject member name
  virobject: Introduce VIR_CLASS_NEW() macro
  virobject: Check if @parent is the first member in class
  cfg.mk: Introduce syntax-check rule to prefer VIR_CLASS_NEW

 cfg.mk  |   8 ++
 src/access/viraccessmanager.c   |   5 +-
 src/bhyve/bhyve_conf.c  |   5 +-
 src/conf/capabilities.c |   5 +-
 src/conf/domain_capabilities.c  |  11 +--
 src/conf/domain_conf.c  |  20 +---
 src/conf/domain_event.c | 166 
 src/conf/network_event.c|  14 +--
 src/conf/node_device_event.c|  21 ++--
 src/conf/object_event.c |  12 +--
 src/conf/secret_event.c |  21 ++--
 src/conf/storage_event.c|  21 ++--
 src/conf/virdomainobjlist.c |   5 +-
 src/conf/virinterfaceobj.c  |  10 +-
 src/conf/virnetworkobj.c|  11 +--
 src/conf/virnodedeviceobj.c |  12 +--
 src/conf/virsecretobj.c |  10 +-
 src/conf/virstorageobj.c|  20 +---
 src/datatypes.c |   7 +-
 src/datatypes.h |  30 +++---
 src/interface/interface_backend_netcf.c |   6 +-
 src/libvirt-admin.c |   7 +-
 src/libvirt-domain-snapshot.c   |   2 +-
 src/libvirt-domain.c|   2 +-
 src/libvirt-host.c  |   2 +-
 src/libvirt-interface.c |   2 +-
 src/libvirt-network.c   |   2 +-
 src/libvirt-nodedev.c   |   8 +-
 src/libvirt-nwfilter.c  |   2 +-
 src/libvirt-secret.c|   2 +-
 src/libvirt-storage.c   |   4 +-
 src/libvirt-stream.c|   2 +-
 src/libxl/libxl_conf.c  |   5 +-
 src/libxl/libxl_domain.c|   5 +-
 src/libxl/libxl_migration.c |   5 +-
 src/logging/log_handler.c   |   5 +-
 src/lxc/lxc_conf.c  |   5 +-
 src/lxc/lxc_monitor.c   |   5 +-
 src/node_device/node_device_driver.c|   4 +-
 src/node_device/node_device_udev.c  |   5 +-
 src/qemu/qemu_agent.c   |   5 +-
 src/qemu/qemu_capabilities.c|   7 +-
 src/qemu/qemu_conf.c|  11 +--
 src/qemu/qemu_domain.c  |  51 +++---
 src/qemu/qemu_monitor.c |   5 +-
 src/remote/remote_daemon_dispatch.c |   4 +-
 src/remote/remote_protocol.x|   2 +-
 src/remote_protocol-structs |   2 +-
 src/rpc/virkeepalive.c  |   5 +-
 src/rpc/virnetclient.c  |   5 +-
 src/rpc/virnetclientprogram.c   |   7 +-
 src/rpc/virnetclientstream.c|   5 +-
 src/rpc/virnetdaemon.c  |   5 +-
 src/rpc/virnetlibsshsession.c   |   5 +-
 src/rpc/virnetsaslcontext.c |  10 +-
 src/rpc/virnetserver.c  |   5 +-
 src/rpc/virnetserverclient.c|   5 +-
 src/rpc/virnetserverprogram.c   |   7 +-
 src/rpc/virnetserverservice.c   |   7 +-
 src/rpc/virnetsocket.c  |   5 +-
 src/rpc/virnetsshsession.c  |   5 +-
 src/rpc/virnettlscontext.c  |  10 +-
 src/security/security_manager.c |   5 +-
 src/test/test_driver.c  |   6 +-
 src/util/virclosecallbacks.c|  11 +--
 src/util/virdnsmasq.c   |   8 +-
 src/util/virfdstream.c  |   5 +-
 src/util/virfilecache.c |   7 +-
 src/util/virhash.c  |  11 +--
 src/util/virhostdev.c   |   5 +-
 src/util/viridentity.c  |   5 +-
 src/util/virmacmap.c|   5 +-
 src/util/virmdev.c  |   5 +-
 src/util/virobject.c|  10 +-
 src/util/virobject.h|   5 +
 src/util/virpci.c   |   5 +-
 src/util/virportallocator.c |   5 +-
 src/util/virresctrl.c   |  10 +-
 src/util/virscsi.c  |   5 +-
 src/util/virscsivhost.c |   5 +-
 src/util/virusb.c   |   5 +-
 src/vbox/vbox_common.c  |   5 +-
 src/vz/vz_driver.c  |   5 +-
 tests/virfilecachetest.c|   7 +-
 84 files changed, 224 insertions(+), 583 deletions(-)

-- 
2.16.1

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

[libvirt] [PATCH v3 1/5] datatypes: Rename @parent to @parentName in virNodeDevice

[Michal Privoznik]

In next patches this name will be needed for a different memeber.
Also, it makes sense to rename the variable because it does not
contain reference to parent device, just its name.

Signed-off-by: Michal Privoznik 
---
 src/conf/virnodedeviceobj.c  | 2 +-
 src/datatypes.c  | 2 +-
 src/datatypes.h  | 2 +-
 src/libvirt-nodedev.c| 6 +++---
 src/node_device/node_device_driver.c | 4 ++--
 src/remote/remote_daemon_dispatch.c  | 4 ++--
 src/remote/remote_protocol.x | 2 +-
 src/remote_protocol-structs  | 2 +-
 src/test/test_driver.c   | 6 +++---
 9 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ad0f27ee47..9d2996046f 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -870,7 +870,7 @@ virNodeDeviceObjListExportCallback(void *payload,
 virNodeDeviceMatch(obj, data->flags)) {
 if (data->devices) {
 if (!(device = virGetNodeDevice(data->conn, def->name)) ||
-VIR_STRDUP(device->parent, def->parent) < 0) {
+VIR_STRDUP(device->parentName, def->parent) < 0) {
 virObjectUnref(device);
 data->error = true;
 goto cleanup;
diff --git a/src/datatypes.c b/src/datatypes.c
index f7eef24ba8..0c3c66a9ce 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -653,7 +653,7 @@ virNodeDeviceDispose(void *obj)
 VIR_DEBUG("release dev %p %s", dev, dev->name);
 
 VIR_FREE(dev->name);
-VIR_FREE(dev->parent);
+VIR_FREE(dev->parentName);
 
 virObjectUnref(dev->conn);
 }
diff --git a/src/datatypes.h b/src/datatypes.h
index 1a8ea01ba3..66733b075c 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -618,7 +618,7 @@ struct _virNodeDevice {
 virObject object;
 virConnectPtr conn; /* pointer back to the connection */
 char *name; /* device name (unique on node) */
-char *parent;   /* parent device name */
+char *parentName;   /* parent device name */
 };
 
 /**
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 563ce889b9..8ced3cea0e 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -346,16 +346,16 @@ virNodeDeviceGetParent(virNodeDevicePtr dev)
 
 virCheckNodeDeviceReturn(dev, NULL);
 
-if (!dev->parent) {
+if (!dev->parentName) {
 if (dev->conn->nodeDeviceDriver && 
dev->conn->nodeDeviceDriver->nodeDeviceGetParent) {
-dev->parent = 
dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);
+dev->parentName = 
dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);
 } else {
 virReportUnsupportedError();
 virDispatchError(dev->conn);
 return NULL;
 }
 }
-return dev->parent;
+return dev->parentName;
 }
 
 
diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 61afa1f8eb..d04a31747a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -256,7 +256,7 @@ nodeDeviceLookupByName(virConnectPtr conn,
 goto cleanup;
 
 if ((device = virGetNodeDevice(conn, name))) {
-if (VIR_STRDUP(device->parent, def->parent) < 0) {
+if (VIR_STRDUP(device->parentName, def->parent) < 0) {
 virObjectUnref(device);
 device = NULL;
 }
@@ -290,7 +290,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn,
 goto cleanup;
 
 if ((device = virGetNodeDevice(conn, def->name))) {
-if (VIR_STRDUP(device->parent, def->parent) < 0) {
+if (VIR_STRDUP(device->parentName, def->parent) < 0) {
 virObjectUnref(device);
 device = NULL;
 }
diff --git a/src/remote/remote_daemon_dispatch.c 
b/src/remote/remote_daemon_dispatch.c
index 5b764bab48..a8a5932d71 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -3805,7 +3805,7 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 parent = virNodeDeviceGetParent(dev);
 
 if (parent == NULL) {
-ret->parent = NULL;
+ret->parentName = NULL;
 } else {
 /* remoteDispatchClientRequest will free this. */
 char **parent_p;
@@ -3815,7 +3815,7 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 VIR_FREE(parent_p);
 goto cleanup;
 }
-ret->parent = parent_p;
+ret->parentName = parent_p;
 }
 
 rv = 0;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 9dbd497b2f..296a087181 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2028,7 +2028,7 @@ struct remote_node_device_get_parent_args {
 };
 
 struct remote_node_device_get_parent_ret {
-

[libvirt] [PATCH v3 5/5] cfg.mk: Introduce syntax-check rule to prefer VIR_CLASS_NEW

[Michal Privoznik]

Now that we have macro that does some checks lets forbid raw
usage of virClassNew() in favor of VIR_CLASS_NEW().

Signed-off-by: Michal Privoznik 
Reviewed-by: Erik Skultety 
---
 cfg.mk | 8 
 1 file changed, 8 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index 4078bc2c63..902178dd1c 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -321,6 +321,11 @@ sc_prohibit_internal_functions:
halt='use VIR_ macros instead of internal functions' \
  $(_sc_search_regexp)
 
+sc_prohibit_raw_virclassnew:
+   @prohibit='virClassNew *\(' \
+   halt='use VIR_CLASS_NEW instead of virClassNew' \
+ $(_sc_search_regexp)
+
 # Avoid raw malloc and free, except in documentation comments.
 sc_prohibit_raw_allocation:
@prohibit='^.[^*].*\<((m|c|re)alloc|free) *\([^)]' \
@@ -1188,6 +1193,9 @@ exclude_file_name_regexp--sc_prohibit_gethostname = 
^src/util/vir(util|log)\.c$$
 exclude_file_name_regexp--sc_prohibit_internal_functions = \
   ^src/(util/(viralloc|virutil|virfile)\.[hc]|esx/esx_vi\.c)$$
 
+exclude_file_name_regexp--sc_prohibit_raw_virclassnew = \
+  ^src/util/virobject\.[hc]$$
+
 exclude_file_name_regexp--sc_prohibit_newline_at_end_of_diagnostic = \
   ^src/rpc/gendispatch\.pl$$
 
-- 
2.16.1

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

[libvirt] [PATCH v3 3/5] virobject: Introduce VIR_CLASS_NEW() macro

[Michal Privoznik]

So far we are repeating the following lines over and over:

  if (!(virSomeObjectClass = virClassNew(virClassForObject(),
 "virSomeObject",
 sizeof(virSomeObject),
 virSomeObjectDispose)))
  return -1;

While this works, it is impossible to do some checking. Firstly,
the class name (the 2nd argument) doesn't match the name in the
code in all cases (the 3rd argument). Secondly, the current style
is needlessly verbose. This commit turns example into following:

  if (!(VIR_CLASS_NEW(virSomeObject,
  virClassForObject)))
  return -1;

Signed-off-by: Michal Privoznik 
---
 src/access/viraccessmanager.c   |   5 +-
 src/bhyve/bhyve_conf.c  |   5 +-
 src/conf/capabilities.c |   5 +-
 src/conf/domain_capabilities.c  |  11 +--
 src/conf/domain_conf.c  |  20 +---
 src/conf/domain_event.c | 166 
 src/conf/network_event.c|  14 +--
 src/conf/node_device_event.c|  21 ++--
 src/conf/object_event.c |  12 +--
 src/conf/secret_event.c |  21 ++--
 src/conf/storage_event.c|  21 ++--
 src/conf/virdomainobjlist.c |   5 +-
 src/conf/virinterfaceobj.c  |  10 +-
 src/conf/virnetworkobj.c|  11 +--
 src/conf/virnodedeviceobj.c |  10 +-
 src/conf/virsecretobj.c |  10 +-
 src/conf/virstorageobj.c|  20 +---
 src/datatypes.c |   5 +-
 src/interface/interface_backend_netcf.c |   6 +-
 src/libvirt-admin.c |   5 +-
 src/libxl/libxl_conf.c  |   5 +-
 src/libxl/libxl_domain.c|   5 +-
 src/libxl/libxl_migration.c |   5 +-
 src/logging/log_handler.c   |   5 +-
 src/lxc/lxc_conf.c  |   5 +-
 src/lxc/lxc_monitor.c   |   5 +-
 src/node_device/node_device_udev.c  |   5 +-
 src/qemu/qemu_agent.c   |   5 +-
 src/qemu/qemu_capabilities.c|   5 +-
 src/qemu/qemu_conf.c|  11 +--
 src/qemu/qemu_domain.c  |  51 +++---
 src/qemu/qemu_monitor.c |   5 +-
 src/rpc/virkeepalive.c  |   5 +-
 src/rpc/virnetclient.c  |   5 +-
 src/rpc/virnetclientprogram.c   |   5 +-
 src/rpc/virnetclientstream.c|   5 +-
 src/rpc/virnetdaemon.c  |   5 +-
 src/rpc/virnetlibsshsession.c   |   5 +-
 src/rpc/virnetsaslcontext.c |  10 +-
 src/rpc/virnetserver.c  |   5 +-
 src/rpc/virnetserverclient.c|   5 +-
 src/rpc/virnetserverprogram.c   |   5 +-
 src/rpc/virnetserverservice.c   |   5 +-
 src/rpc/virnetsocket.c  |   5 +-
 src/rpc/virnetsshsession.c  |   5 +-
 src/rpc/virnettlscontext.c  |  10 +-
 src/security/security_manager.c |   5 +-
 src/util/virclosecallbacks.c|  11 +--
 src/util/virdnsmasq.c   |   6 +-
 src/util/virfdstream.c  |   5 +-
 src/util/virfilecache.c |   5 +-
 src/util/virhash.c  |  11 +--
 src/util/virhostdev.c   |   5 +-
 src/util/viridentity.c  |   5 +-
 src/util/virmacmap.c|   5 +-
 src/util/virmdev.c  |   5 +-
 src/util/virobject.c|  10 +-
 src/util/virobject.h|   4 +
 src/util/virpci.c   |   5 +-
 src/util/virportallocator.c |   5 +-
 src/util/virresctrl.c   |  10 +-
 src/util/virscsi.c  |   5 +-
 src/util/virscsivhost.c |   5 +-
 src/util/virusb.c   |   5 +-
 src/vbox/vbox_common.c  |   5 +-
 src/vz/vz_driver.c  |   5 +-
 tests/virfilecachetest.c|   5 +-
 67 files changed, 167 insertions(+), 535 deletions(-)

diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
index c268ec57f7..b048a367e3 100644
--- a/src/access/viraccessmanager.c
+++ b/src/access/viraccessmanager.c
@@ -54,10 +54,7 @@ static void virAccessManagerDispose(void *obj);
 
 static int virAccessManagerOnceInit(void)
 {
-if (!(virAccessManagerClass = virClassNew(virClassForObjectLockable(),
-  "virAccessManagerClass",
-  sizeof(virAccessManager),
-  virAccessManagerDispose)))
+if (!VIR_CLASS_NEW(virAccessManager, virClassForObjectLockable()))
 return -1;
 
 return 0;
diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c
index b0b40c5754..153de7b391 100644
--- a/src/bhyve/bhyve_conf.c

[libvirt] [PATCH v3 2/5] src: Unify virObject member name

[Michal Privoznik]

Whenever we declare a new object the first member of the struct
has to be virObject (or any other member of that family). Now, up
until now we did not care about the name of the struct member.
But lets unify it so that we can do some checks at compile time
later.

The unified name is 'parent'.

Signed-off-by: Michal Privoznik 
Reviewed-by: Erik Skultety 
---
 src/datatypes.h   | 28 ++--
 src/libvirt-admin.c   |  2 +-
 src/libvirt-domain-snapshot.c |  2 +-
 src/libvirt-domain.c  |  2 +-
 src/libvirt-host.c|  2 +-
 src/libvirt-interface.c   |  2 +-
 src/libvirt-network.c |  2 +-
 src/libvirt-nodedev.c |  2 +-
 src/libvirt-nwfilter.c|  2 +-
 src/libvirt-secret.c  |  2 +-
 src/libvirt-storage.c |  4 ++--
 src/libvirt-stream.c  |  2 +-
 src/qemu/qemu_capabilities.c  |  2 +-
 src/rpc/virnetclientprogram.c |  2 +-
 src/rpc/virnetserverprogram.c |  2 +-
 src/rpc/virnetserverservice.c |  2 +-
 src/util/virdnsmasq.c |  2 +-
 src/util/virfilecache.c   |  2 +-
 tests/virfilecachetest.c  |  2 +-
 19 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/datatypes.h b/src/datatypes.h
index 66733b075c..192c86be80 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -453,7 +453,7 @@ struct _virAdmConnectCloseCallbackData {
  * Internal structure associated to a connection
  */
 struct _virConnect {
-virObjectLockable object;
+virObjectLockable parent;
 
 /* All the variables from here, until declared otherwise in one of
  * the following comments, are setup at time of connection open
@@ -496,7 +496,7 @@ struct _virConnect {
  * Internal structure associated to an admin connection
  */
 struct _virAdmConnect {
-virObjectLockable object;
+virObjectLockable parent;
 virURIPtr uri;
 
 void *privateData;
@@ -512,7 +512,7 @@ struct _virAdmConnect {
  * Internal structure associated to a daemon server
  */
 struct _virAdmServer {
-virObject object;
+virObject parent;
 virAdmConnectPtr conn;  /* pointer back to the admin connection */
 char *name; /* the server external name */
 };
@@ -523,7 +523,7 @@ struct _virAdmServer {
  * Internal structure associated to a client connected to daemon
  */
 struct _virAdmClient {
-virObject object;
+virObject parent;
 virAdmServerPtr srv;/* pointer to the server client is
  * connected to, which also holds a
  * reference back to the admin connection
@@ -539,7 +539,7 @@ struct _virAdmClient {
 * Internal structure associated to a domain
 */
 struct _virDomain {
-virObject object;
+virObject parent;
 virConnectPtr conn;  /* pointer back to the connection */
 char *name;  /* the domain external name */
 int id;  /* the domain ID */
@@ -552,7 +552,7 @@ struct _virDomain {
 * Internal structure associated to a domain
 */
 struct _virNetwork {
-virObject object;
+virObject parent;
 virConnectPtr conn;  /* pointer back to the connection */
 char *name;  /* the network external name */
 unsigned char uuid[VIR_UUID_BUFLEN]; /* the network unique identifier */
@@ -564,7 +564,7 @@ struct _virNetwork {
 * Internal structure associated to a physical host interface
 */
 struct _virInterface {
-virObject object;
+virObject parent;
 virConnectPtr conn;  /* pointer back to the connection */
 char *name;  /* the network external name */
 char *mac;   /* the interface MAC address */
@@ -576,7 +576,7 @@ struct _virInterface {
 * Internal structure associated to a storage pool
 */
 struct _virStoragePool {
-virObject object;
+virObject parent;
 virConnectPtr conn;  /* pointer back to the connection */
 char *name;  /* the storage pool external name */
 unsigned char uuid[VIR_UUID_BUFLEN]; /* the storage pool unique identifier 
*/
@@ -595,7 +595,7 @@ struct _virStoragePool {
 * Internal structure associated to a storage volume
 */
 struct _virStorageVol {
-virObject object;
+virObject parent;
 virConnectPtr conn;  /* pointer back to the connection */
 char *pool;  /* Pool name of owner */
 char *name;  /* the storage vol external name */
@@ -615,7 +615,7 @@ struct _virStorageVol {
  * Internal structure associated with a node device
  */
 struct _virNodeDevice {
-virObject object;
+virObject parent;
 virConnectPtr conn; /* pointer back to the connection */
 char *name; /* device name (unique on node) */
 char *parentName;

[libvirt] [PATCH v4 4/5] qemu: Escape commas for qemuBuildGraphicsVNCCommandLine

[Sukrit Bhatnagar]

Add comma escaping for cfg->vncTLSx509certdir.

Signed-off-by: Sukrit Bhatnagar 
---
 src/qemu/qemu_command.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e903491..8f0dddf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7871,10 +7871,13 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr 
cfg,
 
 if (cfg->vncTLS) {
 virBufferAddLit(, ",tls");
-if (cfg->vncTLSx509verify)
-virBufferAsprintf(, ",x509verify=%s", cfg->vncTLSx509certdir);
-else
-virBufferAsprintf(, ",x509=%s", cfg->vncTLSx509certdir);
+if (cfg->vncTLSx509verify) {
+virBufferAddLit(, ",x509verify=");
+virQEMUBuildBufferEscapeComma(, cfg->vncTLSx509certdir);
+} else {
+virBufferAddLit(, ",x509=");
+virQEMUBuildBufferEscapeComma(, cfg->vncTLSx509certdir);
+}
 }
 
 if (cfg->vncSASL) {
-- 
1.8.3.1

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

[libvirt] [PATCH v4 0/5] qemu: add virQEMUBuildBufferEscapeComma in qemu_command.c

[Sukrit Bhatnagar]

Changes in v4:
Changes made in v2 anbd v3 to qemu_command.c are discarded.
Some changes introduced in v2 are used to create new smaller patches.
virQEMUBuildBufferEscapeComma was applied to:
- info->romfile in qemuBuildRomStr
- disk->vendor and disk->product in qemuBuildDriveDevStr
- fs->src->path in qemuBuildFSStr
- fs->dst in qemuBuildFSDevStr
- cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
- loader->path and loader->nvram in qemuBuildDomainLoaderCommandLine


Changes in v3:
virQEMUBuildBufferEscapeComma was applied to:
- src->hosts->socket in qemuBuildNetworkDriveURI
- src->path, src->configFile in qemuBuildNetworkDriveStr
- disk->blkdeviotune.group_name in qemuBuildDiskThrottling
- net->data.socket.address, net->data.socket.localaddr in
  qemuBuildHostNetStr
- dev->data.file.path in qemuBuildChrChardevStr
- graphics->data.spice.rendernode in
  qemuBuildGraphicsSPICECommandLine
- smartcard->data.cert.file[i], smartcard->data.cert.database in
  qemuBuildSmartcardCommandLine

Link to v3: https://www.redhat.com/archives/libvir-list/2018-April/msg00053.html

Changes in v2:
virQEMUBuildBufferEscapeComma was applied to:
- info->romfile in qemuBuildRomStr
- disk->vendor, disk->product in qemuBuildDriveDevStr
- fs->src->path in qemuBuildFSStr
- fs->dst in qemuBuildFSDevStr
- connect= in qemuBuildHostNetStr
- fileval handling in qemuBuildChrChardevStr
- TYPE_DEV, TYPE_PIPE handling in qemuBuildChrChardevStr
- cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
- cfg->spiceTLSx509certdir in qemuBuildGraphicsSPICECommandLine
- loader->path, loader->nvram usage in
  qemuBuildDomainLoaderCommandLine

Link to v2: https://www.redhat.com/archives/libvir-list/2018-March/msg00965.html


Sukrit Bhatnagar (5):
  qemu: Escape commas for qemuBuildRomStr
  qemu: Escape commas for qemuBuildDriveDevStr
  qemu: Escape commas for qemuBuildFSStr and qemuBuildFSDevStr
  qemu: Escape commas for qemuBuildGraphicsVNCCommandLine
  qemu: Escape commas for qemuBuildDomainLoaderCommandLine

 src/qemu/qemu_command.c | 47 +--
 1 file changed, 29 insertions(+), 18 deletions(-)

-- 
1.8.3.1

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

[libvirt] [PATCH v4 1/5] qemu: Escape commas for qemuBuildRomStr

[Sukrit Bhatnagar]

Add comma escaping for info->romfile.

Signed-off-by: Sukrit Bhatnagar 
---
 src/qemu/qemu_command.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f864350..e7b8aa3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -459,8 +459,10 @@ qemuBuildRomStr(virBufferPtr buf,
 default:
 break;
 }
-if (info->romfile)
-   virBufferAsprintf(buf, ",romfile=%s", info->romfile);
+if (info->romfile) {
+   virBufferAddLit(buf, ",romfile=");
+   virQEMUBuildBufferEscapeComma(buf, info->romfile);
+}
 }
 return 0;
 }
-- 
1.8.3.1

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

[libvirt] [jenkins-ci PATCH 3/3] jobs: Drop autotools-mingw-job

[Andrea Bolognani]

Now that we have variants and we've removed all uses of custom
environment variables, we can convert all jobs that use the
autotools-mingw-job template to the autotools-build-job plus
a few overrides.

As a consequence of this, mingw32 and mingw64 builds will be
split into separate jobs.

Signed-off-by: Andrea Bolognani 
---
 jobs/autotools.yaml| 80 --
 jobs/defaults.yaml | 16 ++
 projects/libvirt-glib.yaml | 12 ++-
 projects/libvirt.yaml  | 12 ++-
 projects/virt-viewer.yaml  | 12 ++-
 5 files changed, 49 insertions(+), 83 deletions(-)

diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
index ac7099f..9868573 100644
--- a/jobs/autotools.yaml
+++ b/jobs/autotools.yaml
@@ -179,83 +179,3 @@
   recipients: '{obj:spam}'
   notify-every-unstable-build: true
   send-to-individuals: false
-
-- job-template:
-id: autotools-mingw-job
-name: '{name}-{branch}-mingw{variant}'
-project-type: matrix
-description: '{title} MinGW'
-autogen_args: ''
-workspace: '{name}-{branch}-mingw{variant}'
-child-workspace: '.'
-block-downstream: true
-block-upstream: true
-wrappers:
-  - timeout:
-  abort: true
-  type: absolute
-  timeout: 90
-  write-description: 'Aborted build after 90 minutes'
-properties:
-  - build-discarder:
-  days-to-keep: 30
-  num-to-keep: 1000
-scm:
-  - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
-  branches:
-- origin/{branch}
-  clean:
-after: true
-  skip-tag: true
-  wipe-workspace: false
-triggers:
-  - reverse:
-  jobs: '{obj:parent_jobs}'
-  - pollscm:
-  cron: "H/20 * * * *"
-axes:
-  - axis:
-  name: systems
-  type: slave
-  values: '{obj:machines}'
-builders:
-  - shell: |
-  {global_env}
-  {local_env}
-  # The MinGW build needs to use the MinGW compiler toolchain,
-  # while $CC is pointing to the native toolchain, so we have
-  # to unset it here.
-  export CC=
-
-  export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw"
-  export PKG_CONFIG_PATH="$VIRT_PREFIX/lib/pkgconfig"
-  export 
PKG_CONFIG_LIBDIR="/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/i686-w64-mingw32/sys-root/mingw/share/pkgconfig"
-
-  mkdir -p build32
-  cd build32
-
-  ../autogen.sh --prefix=$VIRT_PREFIX --host=i686-w64-mingw32
-  $MAKE -j{smp}
-  $MAKE -j{smp} install
-  - shell: |
-  {global_env}
-  {local_env}
-  # See above
-  export CC=
-
-  export VIRT_PREFIX="$VIRT_PREFIX/x86_64-w64-mingw32/sys-root/mingw"
-  export PKG_CONFIG_PATH="$VIRT_PREFIX/lib/pkgconfig"
-  export 
PKG_CONFIG_LIBDIR="/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/x86_64-w64-mingw32/sys-root/mingw/share/pkgconfig"
-
-  mkdir -p build64
-  cd build64
-
-  ../autogen.sh --prefix=$VIRT_PREFIX --host=x86_64-w64-mingw32
-  $MAKE -j{smp}
-  $MAKE -j{smp} install
-publishers:
-  - email:
-  recipients: '{obj:spam}'
-  notify-every-unstable-build: true
-  send-to-individuals: false
diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml
index eef92e8..5527546 100644
--- a/jobs/defaults.yaml
+++ b/jobs/defaults.yaml
@@ -6,5 +6,21 @@
 node: libvirt
 global_env: |
 local_env: |
+mingw32_local_env: |
+  # The MinGW build needs to use the MinGW compiler toolchain,
+  # while $CC is pointing to the native toolchain, so we have
+  # to unset it here.
+  export CC=
+  export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw"
+  export PKG_CONFIG_PATH="$VIRT_PREFIX/lib/pkgconfig"
+  export 
PKG_CONFIG_LIBDIR="/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/i686-w64-mingw32/sys-root/mingw/share/pkgconfig"
+mingw32_autogen_args: --host=i686-w64-mingw32
+mingw64_local_env: |
+  # See above
+  export CC=
+  export VIRT_PREFIX="$VIRT_PREFIX/x86_64-w64-mingw32/sys-root/mingw"
+  export PKG_CONFIG_PATH="$VIRT_PREFIX/lib/pkgconfig"
+  export 
PKG_CONFIG_LIBDIR="/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/x86_64-w64-mingw32/sys-root/mingw/share/pkgconfig"
+mingw64_autogen_args: --host=x86_64-w64-mingw32
 smp: 3
 spam: yman...@redhat.com libvirt...@redhat.com
diff --git a/projects/libvirt-glib.yaml b/projects/libvirt-glib.yaml
index 3873c40..c56e5d3 100644
--- a/projects/libvirt-glib.yaml
+++ b/projects/libvirt-glib.yaml
@@ -26,7 +26,17 @@
 - libvirt-fedora-26
 - libvirt-fedora-27
 - libvirt-fedora-rawhide
-  - autotools-mingw-job:
+  - autotools-build-job:
+  

[libvirt] [PATCH v4 5/5] qemu: Escape commas for qemuBuildDomainLoaderCommandLine

[Sukrit Bhatnagar]

Add comma escaping for loader->path and loader->nvram.

Signed-off-by: Sukrit Bhatnagar 
---
 src/qemu/qemu_command.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8f0dddf..49dc95b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9542,9 +9542,9 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
  NULL);
 }
 
-virBufferAsprintf(,
-  "file=%s,if=pflash,format=raw,unit=%d",
-  loader->path, unit);
+virBufferAddLit(, "file=");
+virQEMUBuildBufferEscapeComma(, loader->path);
+virBufferAsprintf(, ",if=pflash,format=raw,unit=%d", unit);
 unit++;
 
 if (loader->readonly) {
@@ -9557,9 +9557,9 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
 
 if (loader->nvram) {
 virBufferFreeAndReset();
-virBufferAsprintf(,
-  "file=%s,if=pflash,format=raw,unit=%d",
-  loader->nvram, unit);
+virBufferAddLit(, "file=");
+virQEMUBuildBufferEscapeComma(, loader->nvram);
+virBufferAsprintf(, ",if=pflash,format=raw,unit=%d", unit);
 
 virCommandAddArg(cmd, "-drive");
 virCommandAddArgBuffer(cmd, );
-- 
1.8.3.1

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

[libvirt] [PATCH v4 3/5] qemu: Escape commas for qemuBuildFSStr and qemuBuildFSDevStr

[Sukrit Bhatnagar]

Add comma escaping for fs->src->path and fs->dst.

Signed-off-by: Sukrit Bhatnagar 
---
 src/qemu/qemu_command.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b75e441..e903491 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2396,7 +2396,8 @@ qemuBuildFSStr(virDomainFSDefPtr fs,
 }
 
 virBufferAsprintf(, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, 
fs->info.alias);
-virBufferAsprintf(, ",path=%s", fs->src->path);
+virBufferAddLit(, ",path=");
+virQEMUBuildBufferEscapeComma(, fs->src->path);
 
 if (fs->readonly) {
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_READONLY)) {
@@ -2441,7 +2442,8 @@ qemuBuildFSDevStr(const virDomainDef *def,
 virBufferAsprintf(, ",id=%s", fs->info.alias);
 virBufferAsprintf(, ",fsdev=%s%s",
   QEMU_FSDEV_HOST_PREFIX, fs->info.alias);
-virBufferAsprintf(, ",mount_tag=%s", fs->dst);
+virBufferAddLit(, ",mount_tag=");
+virQEMUBuildBufferEscapeComma(, fs->dst);
 
 if (qemuBuildVirtioOptionsStr(, fs->virtio, qemuCaps) < 0)
 goto error;
-- 
1.8.3.1

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

[libvirt] [PATCH v4 2/5] qemu: Escape commas for qemuBuildDriveDevStr

[Sukrit Bhatnagar]

Add comma escaping for disk->vendor and disk->product when being
built for the command line (and not from hotplug).

Signed-off-by: Sukrit Bhatnagar 
---
 src/qemu/qemu_command.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e7b8aa3..b75e441 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2151,11 +2151,15 @@ qemuBuildDriveDevStr(const virDomainDef *def,
 virBufferAsprintf(, ",wwn=0x%s", disk->wwn);
 }
 
-if (disk->vendor)
-virBufferAsprintf(, ",vendor=%s", disk->vendor);
+if (disk->vendor) {
+virBufferAddLit(, ",vendor=");
+virQEMUBuildBufferEscapeComma(, disk->vendor);
+}
 
-if (disk->product)
-virBufferAsprintf(, ",product=%s", disk->product);
+if (disk->product) {
+virBufferAddLit(, ",product=");
+virQEMUBuildBufferEscapeComma(, disk->product);
+}
 
 if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_STORAGE_REMOVABLE)) {
-- 
1.8.3.1

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

[libvirt] [jenkins-ci PATCH 0/3] Introduce variants and use them for MinGW

[Andrea Bolognani]

Andrea Bolognani (3):
  jobs: Introduce variants
  jobs: Tweak autotools-mingw-job template
  jobs: Drop autotools-mingw-job

 jobs/autotools.yaml| 94 --
 jobs/defaults.yaml | 17 +
 jobs/generic.yaml  | 16 
 jobs/go.yaml   |  8 ++--
 jobs/perl-makemaker.yaml   | 12 +++---
 jobs/perl-modulebuild.yaml | 12 +++---
 jobs/python-distutils.yaml | 12 +++---
 projects/libvirt-glib.yaml | 12 +-
 projects/libvirt.yaml  | 12 +-
 projects/virt-viewer.yaml  | 12 +-
 10 files changed, 88 insertions(+), 119 deletions(-)

-- 
2.14.3

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

[libvirt] [jenkins-ci PATCH 1/3] jobs: Introduce variants

[Andrea Bolognani]

This optional feature will allow us to reuse existing job
templates for things like MinGW or website builds.

Signed-off-by: Andrea Bolognani 
---
 jobs/autotools.yaml| 20 ++--
 jobs/defaults.yaml |  1 +
 jobs/generic.yaml  | 16 
 jobs/go.yaml   |  8 
 jobs/perl-makemaker.yaml   | 12 ++--
 jobs/perl-modulebuild.yaml | 12 ++--
 jobs/python-distutils.yaml | 12 ++--
 7 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
index 0c164d3..5c78e6a 100644
--- a/jobs/autotools.yaml
+++ b/jobs/autotools.yaml
@@ -1,11 +1,11 @@
 
 - job-template:
 id: autotools-build-job
-name: '{name}-{branch}-build'
+name: '{name}-{branch}-build{variant}'
 project-type: matrix
 description: '{title} Build'
 autogen_args: ''
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -55,10 +55,10 @@
 
 - job-template:
 id: autotools-syntax-check-job
-name: '{name}-{branch}-syntax-check'
+name: '{name}-{branch}-syntax-check{variant}'
 project-type: matrix
 description: '{title} Syntax Check'
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -94,10 +94,10 @@
 
 - job-template:
 id: autotools-check-job
-name: '{name}-{branch}-check'
+name: '{name}-{branch}-check{variant}'
 project-type: matrix
 description: '{title} Check'
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -137,11 +137,11 @@
 
 - job-template:
 id: autotools-rpm-job
-name: '{name}-{branch}-rpm'
+name: '{name}-{branch}-rpm{variant}'
 project-type: matrix
 description: '{title} RPM'
 archive_format: gz
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -182,11 +182,11 @@
 
 - job-template:
 id: autotools-mingw-job
-name: '{name}-{branch}-mingw'
+name: '{name}-{branch}-mingw{variant}'
 project-type: matrix
 description: '{title} MinGW'
 autogen_args: ''
-workspace: '{name}-{branch}-mingw'
+workspace: '{name}-{branch}-mingw{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml
index 23f8555..eef92e8 100644
--- a/jobs/defaults.yaml
+++ b/jobs/defaults.yaml
@@ -2,6 +2,7 @@
 - defaults:
 name: global
 branch: master
+variant: ''
 node: libvirt
 global_env: |
 local_env: |
diff --git a/jobs/generic.yaml b/jobs/generic.yaml
index 08ab104..f64dde0 100644
--- a/jobs/generic.yaml
+++ b/jobs/generic.yaml
@@ -1,11 +1,11 @@
 
 - job-template:
 id: generic-build-job
-name: '{name}-{branch}-build'
+name: '{name}-{branch}-build{variant}'
 project-type: matrix
 description: '{title} Build'
 autogen_args: ''
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -51,10 +51,10 @@
 
 - job-template:
 id: generic-syntax-check-job
-name: '{name}-{branch}-syntax-check'
+name: '{name}-{branch}-syntax-check{variant}'
 project-type: matrix
 description: '{title} Syntax Check'
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -89,10 +89,10 @@
 
 - job-template:
 id: generic-check-job
-name: '{name}-{branch}-check'
+name: '{name}-{branch}-check{variant}'
 project-type: matrix
 description: '{title} Check'
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
@@ -127,11 +127,11 @@
 
 - job-template:
 id: generic-rpm-job
-name: '{name}-{branch}-rpm'
+name: '{name}-{branch}-rpm{variant}'
 project-type: matrix
 description: '{title} RPM'
 archive_format: gz
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: true
diff --git a/jobs/go.yaml b/jobs/go.yaml
index 2634cb2..9a349ca 100644
--- a/jobs/go.yaml
+++ b/jobs/go.yaml
@@ -1,11 +1,11 @@
 
 - job-template:
 id: go-build-job
-name: '{name}-{branch}-build'
+name: '{name}-{branch}-build{variant}'
 project-type: matrix
 description: '{title} Build'
 autogen_args: ''
-workspace: '{name}-{branch}'
+workspace: '{name}-{branch}{variant}'
 child-workspace: '.'
 block-downstream: true
 block-upstream: 

[libvirt] [jenkins-ci PATCH 2/3] jobs: Tweak autotools-mingw-job template

[Andrea Bolognani]

Make it more similar to the autotools-build-job by dropping the
custom $PREFIX variable and redefining the standard $VIRT_PREFIX
instead, which also makes $PKG_CONFIG_PATH shorter, and moving
all environment variables together.

Signed-off-by: Andrea Bolognani 
---
 jobs/autotools.yaml | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
index 5c78e6a..ac7099f 100644
--- a/jobs/autotools.yaml
+++ b/jobs/autotools.yaml
@@ -228,13 +228,14 @@
   # to unset it here.
   export CC=
 
+  export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw"
+  export PKG_CONFIG_PATH="$VIRT_PREFIX/lib/pkgconfig"
+  export 
PKG_CONFIG_LIBDIR="/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/i686-w64-mingw32/sys-root/mingw/share/pkgconfig"
+
   mkdir -p build32
   cd build32
 
-  export 
PKG_CONFIG_LIBDIR="/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/i686-w64-mingw32/sys-root/mingw/share/pkgconfig"
-  export 
PKG_CONFIG_PATH="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig" \
-  export PREFIX=$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw
-  ../autogen.sh --host=i686-w64-mingw32 --prefix=$PREFIX
+  ../autogen.sh --prefix=$VIRT_PREFIX --host=i686-w64-mingw32
   $MAKE -j{smp}
   $MAKE -j{smp} install
   - shell: |
@@ -243,13 +244,14 @@
   # See above
   export CC=
 
+  export VIRT_PREFIX="$VIRT_PREFIX/x86_64-w64-mingw32/sys-root/mingw"
+  export PKG_CONFIG_PATH="$VIRT_PREFIX/lib/pkgconfig"
+  export 
PKG_CONFIG_LIBDIR="/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/x86_64-w64-mingw32/sys-root/mingw/share/pkgconfig"
+
   mkdir -p build64
   cd build64
 
-  export 
PKG_CONFIG_LIBDIR="/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/x86_64-w64-mingw32/sys-root/mingw/share/pkgconfig"
-  export 
PKG_CONFIG_PATH="$VIRT_PREFIX/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig" \
-  export PREFIX=$VIRT_PREFIX/x86_64-w64-mingw32/sys-root/mingw
-  ../autogen.sh --host=x86_64-w64-mingw32 --prefix=$PREFIX
+  ../autogen.sh --prefix=$VIRT_PREFIX --host=x86_64-w64-mingw32
   $MAKE -j{smp}
   $MAKE -j{smp} install
 publishers:
-- 
2.14.3

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

Re: [libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU

[Collin Walling]

On 04/17/2018 04:18 AM, Jiri Denemark wrote:
> On Mon, Apr 16, 2018 at 19:16:05 -0400, Collin Walling wrote:
>> [Overview]
>>
>> This patch series implements an interface to "query-cpu-model-comparison"
>> (available QEMU ~2.8.0) via virsh cpu-compare.
>>
>> [Using This Feature]
>>
>> Run virsh cpu-compare (ensure you are running the virsh in your build dir) 
>> and
>> pass it an xml file describing a cpu definition. You can copy the cpu xml 
>> from
>> virsh capabilities (if you want to compare the host cpu to itself), or a cpu 
>> defined in any guest xml. Additionally, you can create a cpu xml as such 
>> (e.g.
>> for s390x):
>>
>> 
>> s390x
>> model_name
>> 
>> 
>>
>> NOTE: the presence of  is optional and it will treat the cpu defined 
>> in 
>>   the xml as a host cpu. This will disregard all feature policies (i.e. 
>>   all features listed will behave with policy='require', even if disable 
>>   is specified).
>>
>> NOTE: as s390x only supports feature policies 'require' and 'disable', I am
>>   uncertain how to handle the other policies when parsing CPUDef to JSON.
>>
>> [Example Output]
>>
>> On an s390x system running a z13.2, this is the expected output (where each 
>> file
>> describes a CPU model corresponding to the name of the file):
>>
>> $ virsh cpu-compare zEC12.xml
>> Host CPU is a superset of CPU described in zEC12.xml
>>
>> $ virsh cpu-compare z13.2.xml
>> CPU described in z13.2.xml is identical to host CPU
>>
>> $ virsh cpu-compare z14.xml
>> CPU described in z14.xml is incompatible with host CPU
>>
>> $ virsh cpu-compare z14.xml --error
>> error: Failed to compare host CPU with z14.xml
>> error: the CPU is incompatible with host CPU
>>
>> $ virsh cpu-compare z12345.xml 
>> error: Failed to compare host CPU with z12345cpu.xml
>> error: internal error: unable to execute QEMU command 
>> 'query-cpu-model-comparison': The CPU definition 'z12345-base' is unknown.
>>
>> [Patch Rundown]
>>
>> The first patch copies the host CPU definition from qemuCaps to virCaps so
>> we can easily access the host CPU model and features when we handle the CPU
>> comparison in qemu_driver. Note that we take care to not clobber anything
>> already stored in the host CPU definition until we have successfully 
>> constructed a new host CPU definition.
> 
> I think this is a wrong approach. You'd be basically giving random
> answers depending on which QEMU binary is probed first. The reason for
> storing the CPU model in qemuCaps is that it is tight to a particular
> QEMU binary rather than the host itself. The model reported by one
> binary may not be usable with other binaries and this applies to any
> comparisons or other operations done with this CPU model.
> 
> In other words, we need to introduce a new set of CPU related APIs which
> will take more arguments so that the caller may specify what binary,
> virt type, and machine type they want to use. In other words, the APIs
> should support parameters similar to virConnectGetDomainCapabilities().
> 
> I'm currently starting to work on these new APIs.
> 
> Jirka

I see your concern.

I understand your points behind having multiple arguments to finely control
which qemu we probe, but what do you think of the current code within
"virQEMUCapsInitGuest"? If I understand it correctly, then it has a way of 
querying the "native qemu binary" capabilities (e.g. qemu-kvm).

We could refactor this code to get these "kvmbinCaps" when we need it, and
from that we can retrieve the host CPU model. We would not need to specify
a binary for this, as we already have a list of "native binaries" that we can
test. As for virt type, we can still specify this via "virQEMUCapsGetHostModel".

I think that would suffice, at least enough for what this patch series needs.
I could spin up a patch for this if you'd like and we can see if it makes 
sense?

Just some of my thoughts, and thanks for your response.

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


-- 
Respectfully,
- Collin Walling

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

Re: [libvirt] [PATCH v4 05/14] qemu: Store pr runtime data in status XML

[Michal Privoznik]

On 04/17/2018 05:07 PM, John Ferlan wrote:
> 
> 
> On 04/17/2018 10:19 AM, Michal Privoznik wrote:
>> On 04/17/2018 02:00 PM, John Ferlan wrote:
>>>
>>>
>>> On 04/16/2018 10:56 AM, Michal Privoznik wrote:
 On 04/13/2018 10:57 PM, John Ferlan wrote:
>
>
> On 04/10/2018 10:58 AM, Michal Privoznik wrote:
>> Now that we generate pr-manager alias and socket path store them
>> in status XML so that they are preserved across daemon restarts.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>  src/qemu/qemu_domain.c | 64 
>> ++
>>  1 file changed, 64 insertions(+)
>>
>
> So if we were to save this information (and at this point I don't think
> we need to), then this is where we should be allocating and filling in
> the private data (e.g. not in the previous patch).

 How come? What would be left from the previous patch if private runtime
 struct would be introduced only here? Or are you just suggesting
 swapping these two patches?

>>>
>>> I hope I provided enough feedback in the prior response to answer this.
>>>
>
> Again as I noted previously, save the alias when printing the domain
> active information and I think you're good.

 No, I don't want to expose info on PR helper more than is necessary. The
 fact that it's a separate process should not be visible to users because
 it is an implementation detail of QEMU. Other hypervisors might do this
 differently. And even though it might not be visible from the patches,
 using unmanaged mode is discouraged. In fact, unmanaged mode is on the
 edge. If pr-helper is viewed as internal implementation the unmanaged
 mode has no place in libvirt. However, qemu devels are experimenting
 with systemd socket activation and for socket path must be configurable
 through libvirt. So the only reason for using unmanaged PRs is systemd
 socket activation.
>>>
>>> We "expose" aliases a lot in the active domain XML. Someone that's going
>>> to add a  to their 
>>> definition I cannot believe would be surprised to see an alias printed.
>>
>> We already don't expose some aliases. For instance, if a domain is
>> configured to use hugepages and we use memory-backend-file we don't
>> report generated aliases anywhere. Why? Because the fact we are using
>> memory-backend-file to tell qemu to use hugepages is internal
>> implementation. And users should not be concerned with that. It is the
>> same story with pr-manager and its alias. It is internal implementation
>> deatail and as such we should not expose it.
>>
> 
> Does that code save the alias in some private structure? The correlation
> being it's the libvirt <-> qemu interaction and saving it has
> implications related to what any future change may need to handle.

No it doesn't. Because that devices can't be manipulated. So the alias
is constructed just once, added to the command line and the forgot. But
with pr-manger we need to be able to manipulate it.

> 
>>>
>>> How would they know from the alias that it's a separate process? The
>>> only way to correlate the two would be to read the code and know what
>>> QEMU did to make libvirt do a little dance in order to support.
>>
>> You probably misunderstood what I meant. My idea is to expose as little
>> info back to user as possible in this case. I don't see any compelling
>> reason for user to learn the pr-manager's alias.
>>
>>>
>>> As for systemd, oh great another area to fall flat on our faces...
>>> Wasn't another reason to shorten the path w/ domain name because there
>>> was some sort of bad systemd interaction?
>>
>> Don't recall. It's not relevant.
>>
>>>

 Side note, we are not even exposing qemu's PID anywhere because not
 every hypervisor we support has VMs as separate processes.

>>>
>>> The PID though could be an unexposed domain private data, couldn't it?
>>
>> Why should we track PID of pr-helper? What do we need it for? As Peter
>> pointed out in review to my previous patches, PIDs change therefore if
>> we start pr-helper process with PID X, later when shutting down domain
>> we could find a different process under the same PID. Because pr-helper
>> might have died, released the PID and another process could have been
>> started with the same PID.
>>
> 
> True...  Of course death of pr-helper is another problem not entirely
> managed by any of this IIRC - the assumption being that if we started
> one once, then it'll run forever, but if it does die then we won't
> because we assume that once started is always started. Still, as you
> pointed out elsewhere some sort of future event should help us to
> perform a restart. Leaving libvirt to manage the qemu problem.
> 
> I guess I see pr-helper as a domain private thing ready to start/stop
> when some other disk source code comes along and says I have this thing
> and need to use the managed 

Re: [libvirt] [PATCH v2 3/5] virobject: Introduce VIR_CLASS_NEW() macro

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 05:07:41PM +0200, Michal Privoznik wrote:
> On 04/17/2018 10:32 AM, Daniel P. Berrangé wrote:
> > On Tue, Apr 17, 2018 at 10:20:51AM +0200, Michal Privoznik wrote:
> >> So far we are repeating the following lines over and over:
> >>
> >>   if (!(virSomeObjectClass = virClassNew(virClassForObject(),
> >>  "virSomeObject",
> >>  sizeof(virSomeObject),
> >>  virSomeObjectDispose);
> >>   return -1;
> >>
> >> While this works, it is impossible to do some checking. Firstly,
> >> the class name (the 2nd argument) doesn't match the name in the
> >> code in all cases (the 3rd argument). Secondly, the current style
> >> is needlessly verbose. This commit turns example into following:
> >>
> >>   VIR_CLASS_NEW(virClassForObject(),
> >> virSomeObject);
> >>
> >> Signed-off-by: Michal Privoznik 
> > 
> > [snip]
> > 
> >> diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
> >> index c268ec57f7..a8d361d389 100644
> >> --- a/src/access/viraccessmanager.c
> >> +++ b/src/access/viraccessmanager.c
> >> @@ -54,11 +54,8 @@ static void virAccessManagerDispose(void *obj);
> >>  
> >>  static int virAccessManagerOnceInit(void)
> >>  {
> >> -if (!(virAccessManagerClass = virClassNew(virClassForObjectLockable(),
> >> -  "virAccessManagerClass",
> >> -  sizeof(virAccessManager),
> >> -  virAccessManagerDispose)))
> >> -return -1;
> >> +VIR_CLASS_NEW(virClassForObjectLockable(),
> >> +  virAccessManager);
> > 
> > Ewww, I definitely do not like this approach - it is hiding control
> > flow which can exit the callpath inside a macro which is a big no.
> > It isn't hard to make it work in an explicit way as
> > 
> >if (VIR_CLASS_NEW(virClassForObjectLockable(),
> >  virAccessManager) < 0)
> >   return -1;
> 
> So if VIR_CLASS_NEW() should wrap virClassNew() how come this example
> compares the result with integer? Shouldn't hat be:
> 
>   if (!VIR_CLAS_NEW(..))
> return -1;

Yes, my bad - I had  VIR_ALLOC() on the brain when i mistakenly
wrote < 0 instead of == NULL (or  just !).

> or do you see VIR_CLASS_NEW defined as an expression returning integer,
> e.g. like this:
> 
> # define VIR_CLASS_NEW(name, prnt) \
> ((name##Class = virClassNew(prnt, #name, sizeof(name), name##Dispose)) ? 
> 0 : -1)

No, it was a mistake.


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

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

Re: [libvirt] [PATCH v2 3/5] virobject: Introduce VIR_CLASS_NEW() macro

[Michal Privoznik]

On 04/17/2018 10:32 AM, Daniel P. Berrangé wrote:
> On Tue, Apr 17, 2018 at 10:20:51AM +0200, Michal Privoznik wrote:
>> So far we are repeating the following lines over and over:
>>
>>   if (!(virSomeObjectClass = virClassNew(virClassForObject(),
>>  "virSomeObject",
>>  sizeof(virSomeObject),
>>  virSomeObjectDispose);
>>   return -1;
>>
>> While this works, it is impossible to do some checking. Firstly,
>> the class name (the 2nd argument) doesn't match the name in the
>> code in all cases (the 3rd argument). Secondly, the current style
>> is needlessly verbose. This commit turns example into following:
>>
>>   VIR_CLASS_NEW(virClassForObject(),
>> virSomeObject);
>>
>> Signed-off-by: Michal Privoznik 
> 
> [snip]
> 
>> diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
>> index c268ec57f7..a8d361d389 100644
>> --- a/src/access/viraccessmanager.c
>> +++ b/src/access/viraccessmanager.c
>> @@ -54,11 +54,8 @@ static void virAccessManagerDispose(void *obj);
>>  
>>  static int virAccessManagerOnceInit(void)
>>  {
>> -if (!(virAccessManagerClass = virClassNew(virClassForObjectLockable(),
>> -  "virAccessManagerClass",
>> -  sizeof(virAccessManager),
>> -  virAccessManagerDispose)))
>> -return -1;
>> +VIR_CLASS_NEW(virClassForObjectLockable(),
>> +  virAccessManager);
> 
> Ewww, I definitely do not like this approach - it is hiding control
> flow which can exit the callpath inside a macro which is a big no.
> It isn't hard to make it work in an explicit way as
> 
>if (VIR_CLASS_NEW(virClassForObjectLockable(),
>  virAccessManager) < 0)
>   return -1;

So if VIR_CLASS_NEW() should wrap virClassNew() how come this example
compares the result with integer? Shouldn't hat be:

  if (!VIR_CLAS_NEW(..))
return -1;

or do you see VIR_CLASS_NEW defined as an expression returning integer,
e.g. like this:

# define VIR_CLASS_NEW(name, prnt) \
((name##Class = virClassNew(prnt, #name, sizeof(name), name##Dispose)) ? 0 
: -1)

Michal

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

Re: [libvirt] [dbus PATCH 09/25] Implement BlockPull method for Domain Interface

[Ján Tomko]

On Tue, Apr 17, 2018 at 02:04:28PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  7 +++
src/domain.c| 26 ++
2 files changed, 33 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v4 05/14] qemu: Store pr runtime data in status XML

[John Ferlan]

On 04/17/2018 10:19 AM, Michal Privoznik wrote:
> On 04/17/2018 02:00 PM, John Ferlan wrote:
>>
>>
>> On 04/16/2018 10:56 AM, Michal Privoznik wrote:
>>> On 04/13/2018 10:57 PM, John Ferlan wrote:


 On 04/10/2018 10:58 AM, Michal Privoznik wrote:
> Now that we generate pr-manager alias and socket path store them
> in status XML so that they are preserved across daemon restarts.
>
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_domain.c | 64 
> ++
>  1 file changed, 64 insertions(+)
>

 So if we were to save this information (and at this point I don't think
 we need to), then this is where we should be allocating and filling in
 the private data (e.g. not in the previous patch).
>>>
>>> How come? What would be left from the previous patch if private runtime
>>> struct would be introduced only here? Or are you just suggesting
>>> swapping these two patches?
>>>
>>
>> I hope I provided enough feedback in the prior response to answer this.
>>

 Again as I noted previously, save the alias when printing the domain
 active information and I think you're good.
>>>
>>> No, I don't want to expose info on PR helper more than is necessary. The
>>> fact that it's a separate process should not be visible to users because
>>> it is an implementation detail of QEMU. Other hypervisors might do this
>>> differently. And even though it might not be visible from the patches,
>>> using unmanaged mode is discouraged. In fact, unmanaged mode is on the
>>> edge. If pr-helper is viewed as internal implementation the unmanaged
>>> mode has no place in libvirt. However, qemu devels are experimenting
>>> with systemd socket activation and for socket path must be configurable
>>> through libvirt. So the only reason for using unmanaged PRs is systemd
>>> socket activation.
>>
>> We "expose" aliases a lot in the active domain XML. Someone that's going
>> to add a  to their 
>> definition I cannot believe would be surprised to see an alias printed.
> 
> We already don't expose some aliases. For instance, if a domain is
> configured to use hugepages and we use memory-backend-file we don't
> report generated aliases anywhere. Why? Because the fact we are using
> memory-backend-file to tell qemu to use hugepages is internal
> implementation. And users should not be concerned with that. It is the
> same story with pr-manager and its alias. It is internal implementation
> deatail and as such we should not expose it.
> 

Does that code save the alias in some private structure? The correlation
being it's the libvirt <-> qemu interaction and saving it has
implications related to what any future change may need to handle.

>>
>> How would they know from the alias that it's a separate process? The
>> only way to correlate the two would be to read the code and know what
>> QEMU did to make libvirt do a little dance in order to support.
> 
> You probably misunderstood what I meant. My idea is to expose as little
> info back to user as possible in this case. I don't see any compelling
> reason for user to learn the pr-manager's alias.
> 
>>
>> As for systemd, oh great another area to fall flat on our faces...
>> Wasn't another reason to shorten the path w/ domain name because there
>> was some sort of bad systemd interaction?
> 
> Don't recall. It's not relevant.
> 
>>
>>>
>>> Side note, we are not even exposing qemu's PID anywhere because not
>>> every hypervisor we support has VMs as separate processes.
>>>
>>
>> The PID though could be an unexposed domain private data, couldn't it?
> 
> Why should we track PID of pr-helper? What do we need it for? As Peter
> pointed out in review to my previous patches, PIDs change therefore if
> we start pr-helper process with PID X, later when shutting down domain
> we could find a different process under the same PID. Because pr-helper
> might have died, released the PID and another process could have been
> started with the same PID.
> 

True...  Of course death of pr-helper is another problem not entirely
managed by any of this IIRC - the assumption being that if we started
one once, then it'll run forever, but if it does die then we won't
because we assume that once started is always started. Still, as you
pointed out elsewhere some sort of future event should help us to
perform a restart. Leaving libvirt to manage the qemu problem.

I guess I see pr-helper as a domain private thing ready to start/stop
when some other disk source code comes along and says I have this thing
and need to use the managed domain pr-helper - please add me. The domain
pr-helper code then could say, well this is a first - something needs to
be started too. Using return values you could know failure=-1, new=0,
just-add=1. Keeping track of the number of disks using it in the domain
private structure and when hotunplug causes that count to go to zero, we
can remove the pr-helper. 

Re: [libvirt] [dbus PATCH 08/25] Implement BlockJobAbort method for Domain Interface

[Ján Tomko]

On Tue, Apr 17, 2018 at 02:04:27PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  6 ++
src/domain.c| 25 +
2 files changed, 31 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [dbus PATCH 07/25] Implement BlockCommit method for Domain Interface

[Ján Tomko]

On Tue, Apr 17, 2018 at 02:04:26PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  9 +
src/domain.c| 28 
2 files changed, 37 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH] domain: avoid slash characters to the new domain name.

[Julio Faracco]

Hi Peter,

I read the BZ comments and Cole's suggestion, but I didn't know about
the requirements of the other hypervisors.
Since slash is commonly used for paths.

I will investigate it properly.

--
Julio Cesar Faracco

2018-04-17 4:14 GMT-03:00 Peter Krempa :
>
> On Tue, Apr 17, 2018 at 00:27:27 -0300, Julio Faracco wrote:
> > The 'domrename' command needs to check if the new domain name contains
> > the slash character. This character is not accepted by libvirt XML
> > definition because it is an invalid char (see Cole's commit b1fc6a7b7).
> > This commit enhace the 'domrename' command adding this check.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1333232
> >
> > Signed-off-by: Julio Faracco 
> > ---
> >  src/internal.h   | 9 +
> >  src/libvirt-domain.c | 1 +
> >  2 files changed, 10 insertions(+)
>
> Slash in VM name may be allowed for some hypervisors and when defining
> the hypervisor defines this via VIR_DOMAIN_DEF_FEATURE_NAME_SLASH parser
> feature flag.
>
> Currently it appears that virtualbox, phyp, vmx, and xenapi driver
> support a slash so the fix below is wrong as it would forbid the slash
> when renaming the VM for those drivers.
>
> This needs to be implemented in the qemu driver according to the BZ.

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

Re: [libvirt] [dbus PATCH 06/25] Add AddIOThread method for Domain Interface

[Ján Tomko]

On Tue, Apr 17, 2018 at 02:04:25PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  6 ++
src/domain.c| 25 +
2 files changed, 31 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCHv2 0/4] qemu: enable sandbox whitelist by default

[Ján Tomko]

On Tue, Apr 10, 2018 at 04:49:38PM +0200, Ján Tomko wrote:

v1: https://www.redhat.com/archives/libvir-list/2018-March/msg01965.html
https://bugzilla.redhat.com/show_bug.cgi?id=1492597
v2:
* also deny resource control
* split out and refactor the command line building
* be explicit about denying the obsolete syscalls

Ján Tomko (4):
 Introduce QEMU_CAPS_SECCOMP_BLACKLIST
 Introduce qemuBuildSeccompSandboxCommandLine
 Refactor qemuBuildSeccompSandboxCommandLine
 qemu: deny privilege elevation and spawn in seccomp



Thank you for the reviews, I have rebased the patches to get rid of the
old SECCOMP_SANDBOX capability and pushed the series.

Jano


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

Re: [libvirt] [PATCH v4 07/14] qemu_cgroup: Allow /dev/mapper/control for PR

[Michal Privoznik]

On 04/17/2018 02:25 PM, John Ferlan wrote:
> 
> 
> On 04/16/2018 10:56 AM, Michal Privoznik wrote:
>> On 04/14/2018 02:55 PM, John Ferlan wrote:
>>>
>>>
>>> On 04/10/2018 10:58 AM, Michal Privoznik wrote:
 Just like in previous commit, qemu-pr-helper might want to open
 /dev/mapper/control under certain circumstances. Therefore we
 have to allow it in cgroups.
>>>
>>> Perhaps the two patches should be combined then...  yes, I know cgroups
>>> is different than namespace, so I understand the separation.
>>
>> Yeah, I'd like to keep them separated. Even though they allow access to
>> the same path they do that in different areas of the code.
>>
> 
> Separate is fine...
> 
>>>

 Signed-off-by: Michal Privoznik 
 ---
  src/qemu/qemu_cgroup.c  | 33 ++---
  src/util/virdevmapper.c |  8 +++-
  2 files changed, 37 insertions(+), 4 deletions(-)

 diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
 index d88eb7881f..546a4c8e63 100644
 --- a/src/qemu/qemu_cgroup.c
 +++ b/src/qemu/qemu_cgroup.c
 @@ -114,6 +114,8 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
  }
  
  
 +#define DEVICE_MAPPER_CONTROL_PATH "/dev/mapper/control"
 +
>>>
>>> Almost too bad we didn't have a common place for this in some existing
>>> included .h file.
>>
>> Yeah :(
>>
>>>
  static int
  qemuSetupImageCgroupInternal(virDomainObjPtr vm,
   virStorageSourcePtr src,
 @@ -125,6 +127,10 @@ qemuSetupImageCgroupInternal(virDomainObjPtr vm,
  return 0;
  }
  
 +if (virStoragePRDefIsManaged(src->pr) &&
 +qemuSetupImagePathCgroup(vm, DEVICE_MAPPER_CONTROL_PATH, false) < 
 0)
 +return -1;
 +
>>>
>>> Could the above be done potentially many times?  Could be expensive, no?
>>>  Considering what virDevMapperGetTargets[Impl] does...
>>
>> It shouldn't be expensive. At the very first call of
>> virDevMapperGetTargetsImpl() we get ENXIO (this the change below) and
>> thus there only very small time penalty added.
>>
> 
> Suffice to say it wasn't obvious to me WTF virDevMapperGetTargets[Impl]
> causes the ENXIO.  That logic is not entirely self documenting.
> 
>>>
  return qemuSetupImagePathCgroup(vm, src->path, src->readonly || 
 forceReadonly);
  }
  
 @@ -142,9 +148,8 @@ qemuTeardownImageCgroup(virDomainObjPtr vm,
  virStorageSourcePtr src)
  {
  qemuDomainObjPrivatePtr priv = vm->privateData;
 -int perms = VIR_CGROUP_DEVICE_READ |
 -VIR_CGROUP_DEVICE_WRITE |
 -VIR_CGROUP_DEVICE_MKNOD;
 +int perms = VIR_CGROUP_DEVICE_RWM;
 +size_t i;
  int ret;
  
  if (!virCgroupHasController(priv->cgroup,
 @@ -157,6 +162,28 @@ qemuTeardownImageCgroup(virDomainObjPtr vm,
  return 0;
  }
  
 +for (i = 0; i < vm->def->ndisks; i++) {
 +virStorageSourcePtr diskSrc = vm->def->disks[i]->src;
 +
 +if (src == diskSrc)
 +continue;
 +
 +if (virStoragePRDefIsManaged(diskSrc->pr))
 +break;
 +}
 +
 +if (i == vm->def->ndisks) {
>>>
>>> If there was only one that's managed and it's @src, then we don't get
>>> here...
>>>
>>
>> How come? Say there are 3 disks for a domain and the first one is
>> managed: disks = {A, B, C} (where A.managed = yes}.
>>
> 
> So it made sense when I wrote the comment... let's see.
> 
> 
>> So the loop goes like this:
>>
>>   qemuTeardownImageCgroup(A):
>> i = 0;
>> if (A.src == disks[0].src) //true
>>   continue;
>>
>> i = 1;
>> if (A.src == disks[1].src) //false
>>   continue;
>>
>> if (isManaged(disks[1]) //false
>>   break;
>>
>> i = 2;
>> if (A.src == disks[2].src) //false
>>   continue;
>>
>> if (isManaged(disks[2]) //false
>>   break;
>>
>> // Here, i == ndisks == 3;
>>
>> Or am I missing something?
>>
> 
> OK - with the example I see...  /me wonders at this point whether it'd
> be clearer to keep a list of pr-helper managed LUN's and "Add" and
> "Remove" where the 1st add and the last remove trigger the PR start/stop
> rather than looping through ndisks.

Well, that would possibly allocate much more than 11 bytes ;-)

What we can do, however, when qemu introduces the even support is to
track if pr-helper is running (say, we'd have a bool under
vm->privateData) and doing the following:

if (vm->privateData->prRunning == false && virStoragePRDefIsManaged())
  startPRHelper();

Hm.. in fact we can do something like that now even though it will not
reflect reality 100%. But it's not going to be any worse that what we
have now. Nor better though.

If I will have to send yet another version, I might rework it. But as I
say, it doesn't make any difference now.

Michal

--

Re: [libvirt] [PATCH v4 05/14] qemu: Store pr runtime data in status XML

[Michal Privoznik]

On 04/17/2018 02:00 PM, John Ferlan wrote:
> 
> 
> On 04/16/2018 10:56 AM, Michal Privoznik wrote:
>> On 04/13/2018 10:57 PM, John Ferlan wrote:
>>>
>>>
>>> On 04/10/2018 10:58 AM, Michal Privoznik wrote:
 Now that we generate pr-manager alias and socket path store them
 in status XML so that they are preserved across daemon restarts.

 Signed-off-by: Michal Privoznik 
 ---
  src/qemu/qemu_domain.c | 64 
 ++
  1 file changed, 64 insertions(+)

>>>
>>> So if we were to save this information (and at this point I don't think
>>> we need to), then this is where we should be allocating and filling in
>>> the private data (e.g. not in the previous patch).
>>
>> How come? What would be left from the previous patch if private runtime
>> struct would be introduced only here? Or are you just suggesting
>> swapping these two patches?
>>
> 
> I hope I provided enough feedback in the prior response to answer this.
> 
>>>
>>> Again as I noted previously, save the alias when printing the domain
>>> active information and I think you're good.
>>
>> No, I don't want to expose info on PR helper more than is necessary. The
>> fact that it's a separate process should not be visible to users because
>> it is an implementation detail of QEMU. Other hypervisors might do this
>> differently. And even though it might not be visible from the patches,
>> using unmanaged mode is discouraged. In fact, unmanaged mode is on the
>> edge. If pr-helper is viewed as internal implementation the unmanaged
>> mode has no place in libvirt. However, qemu devels are experimenting
>> with systemd socket activation and for socket path must be configurable
>> through libvirt. So the only reason for using unmanaged PRs is systemd
>> socket activation.
> 
> We "expose" aliases a lot in the active domain XML. Someone that's going
> to add a  to their 
> definition I cannot believe would be surprised to see an alias printed.

We already don't expose some aliases. For instance, if a domain is
configured to use hugepages and we use memory-backend-file we don't
report generated aliases anywhere. Why? Because the fact we are using
memory-backend-file to tell qemu to use hugepages is internal
implementation. And users should not be concerned with that. It is the
same story with pr-manager and its alias. It is internal implementation
deatail and as such we should not expose it.

> 
> How would they know from the alias that it's a separate process? The
> only way to correlate the two would be to read the code and know what
> QEMU did to make libvirt do a little dance in order to support.

You probably misunderstood what I meant. My idea is to expose as little
info back to user as possible in this case. I don't see any compelling
reason for user to learn the pr-manager's alias.

> 
> As for systemd, oh great another area to fall flat on our faces...
> Wasn't another reason to shorten the path w/ domain name because there
> was some sort of bad systemd interaction?

Don't recall. It's not relevant.

> 
>>
>> Side note, we are not even exposing qemu's PID anywhere because not
>> every hypervisor we support has VMs as separate processes.
>>
> 
> The PID though could be an unexposed domain private data, couldn't it?

Why should we track PID of pr-helper? What do we need it for? As Peter
pointed out in review to my previous patches, PIDs change therefore if
we start pr-helper process with PID X, later when shutting down domain
we could find a different process under the same PID. Because pr-helper
might have died, released the PID and another process could have been
started with the same PID.

Michal

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

Re: [libvirt] [PATCH v4 04/14] qemu: Generate alias and socket path for pr-helper

[Michal Privoznik]

On 04/17/2018 01:49 PM, John Ferlan wrote:
> 
> 
> On 04/16/2018 10:56 AM, Michal Privoznik wrote:
>> On 04/13/2018 10:51 PM, John Ferlan wrote:
>>>
>>>
>>> On 04/10/2018 10:58 AM, Michal Privoznik wrote:
 While we're not generating the command line just yet (look for
 the next commits), we can generate the alias for pr-manager.
 A domain can have up to one managed pr-manager (in which case
 socket path is decided by libvirt and pr-helper is spawned by
 libvirt too), but up to ndisks of unmanaged ones (one per disk
 basically). In case of the former we can generate a simple alias
 and be sure it'll not conflict. But in case of the latter to
>>>
>>> Well if it's only one, then it had better not conflict (IOW: after the
>>> and is unnecessary because there is check).
>>>
 avoid any conflicts let's generate alias that's based on
 something unique - like disk target.

>>>
>>> Make sure this commit message follows whatever happens in this patch...
>>>
 Signed-off-by: Michal Privoznik 
 ---
  src/libvirt_private.syms  |  2 ++
  src/qemu/qemu_alias.c | 11 ++
  src/qemu/qemu_alias.h |  2 ++
  src/qemu/qemu_domain.c| 87 
 +--
  src/qemu/qemu_domain.h| 10 ++
  src/util/virstoragefile.c | 15 
  src/util/virstoragefile.h |  2 ++
  7 files changed, 126 insertions(+), 3 deletions(-)

>>>
>>> This patch does two things - one is just the pure alias/path for the
>>> pr-helper for the active domain. The second is copy that same
>>> information to the private storage source, which I'm not sure (yet) why
>>> it's necessary since both can be regenerated as needed based on fairly
>>> static data as opposed to secinfo and encinfo which get filled in with
>>> information only available during Prepare (the interaction with the
>>> secret driver and the need for the @conn pointer) that wasn't available
>>> during launch/command line building. Although some of that has changed
>>> with more recent changes to be able to get a secret conn "on the fly".
>>> Anyway, I digress.
>>>
>>> The point being - please note why you're also saving something in the
>>> private storage source area...  The 'path' is already present in the
>>> domain XML (both active and inactive) and the 'alias' could be saved in
>>> the active output if you tweaked virStoragePRDefFormat to match what
>>> virDomainDeviceInfoFormat does.
>>
>> Okay. I will put it in the commit message. I thought we've discussed
>> this earlier. I rather put and info into status XML even though it might
>> look useless right now than having to write some crazy backcompat code
>> later.
>>
> 
> I don't think this is the same as @channelTargetDir (as you note in the
> next response). The problem there was a reliance on a name that ended up
> being too long and yes, it was a mess. I dunno - I think I've tried the
> future proofing things too and was told it was unnecessary.

Depends on problem you were trying to future proof from.

> 
> Still in order to "provide insight" into my why... Here you have a @path
> that's essentially static outside of the commonly used priv->libDir and
> an @alias that is static when managed. When unmanaged, the @path is
> provided by a consumer and the @alias is essentially a static prefix
> followed by a commonly used source destination alias. So nothing so far
> that would need to be saved in two places.

Not true. If alias were a static string, then _qemuDomainDiskPRD would
need to reflect that. That means, alias must be 'const char *' because
storing a static string into 'char *' is a big NO NO. But at the same
time, alias is dynamically generated (for unmanaged disks), so it has to
be 'char *'.  Sure, we can obfuscate it by inventing new struct member,
but
just look at how ugly it looks:

  struct qemuDomainDiskPRD {
char *alias;
const char *constAlias;
char *path;
  };

Also, think of all those special casing in the code that uses the
struct. This approach that I implemented here is used widely across our
code base: qemuAssignDeviceTPMAlias(), qemuAssignDeviceWatchdogAlias()
to name few.

Why we even need this struct you ask? Couple of reasons:

a) this approach implemented here is specific to QEMU and QEMU only. It
isn't hypervisor agnostic, thus it doesn't belong to _virStoragePRDef.

b) we should separate runtime and config data. That's why virStorage*()
functions live under src/util/ as they work with config and not runtime
(as in hypervisor specific). This merely mimics vm->privateData
abstraction.

> 
> Since @alias is an "internal" libvirt->qemu construct we wouldn't have
> the same problem as some "external" construct changing our definition.
> Unlike perhaps _qemuDomainVcpuPrivate where the alias perhaps changes
> based on ordering, insert, remove, etc. (I didn't look, but made the
> assumption). For PR there is one and it's not going to change. Even 

Re: [libvirt] [PATCH v2 5/5] qemu: Enable memory-backend-file.discard-data whenever possible

[Eduardo Habkost]

On Tue, Apr 17, 2018 at 01:16:42PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1480668
> 
> The cases when we cannot enable this optimization are:
>   1) nvdimms
>   2) if memAccess='shared'

The specific use case for discard-data=on uses share=on, see:
https://bugzilla.redhat.com/show_bug.cgi?id=1460848#c4

It looks like this will require a explicit XML element, after
all.


> 
> Otherwise it is safe to enable it.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 527a35779d..b920f5c3e4 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3121,6 +3121,13 @@ qemuBuildMemoryBackendStr(virJSONValuePtr 
> *backendProps,
>NULL) < 0)
>  goto cleanup;
>  
> +if (!mem->nvdimmPath && memAccess != VIR_DOMAIN_MEMORY_ACCESS_SHARED 
> &&
> +virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD) &&
> +virJSONValueObjectAdd(props,
> +  "B:discard-data", true,
> +  NULL) < 0)
> +goto cleanup;
> +
>  switch (memAccess) {
>  case VIR_DOMAIN_MEMORY_ACCESS_SHARED:
>  if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0)
> -- 
> 2.16.1
> 

-- 
Eduardo

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

Re: [libvirt] [jenkins-ci PATCH v2 3/6] projects: Add libvirt-master-website job

[Daniel P . Berrangé]

On Tue, Apr 17, 2018 at 04:21:19PM +0200, Andrea Bolognani wrote:
> On Tue, 2018-04-17 at 15:02 +0100, Daniel P. Berrangé wrote:
> > On Tue, Apr 17, 2018 at 03:57:04PM +0200, Andrea Bolognani wrote:
> > > We could go one further and use
> > > 
> > >   name: '{name}-{branch}-build{variant}'
> > > 
> > > for the generic-build-job template and
> > > 
> > >   variant: +website
> > > 
> > > for this specific job, then change the autotools-mingw-job
> > > template to use
> > > 
> > >   name: '{name}-{branch}-build+mingw'
> > > 
> > > to align the job names with the project names at the Ansible
> > > level.
> > 
> > Yes, that would be reasonable, though I prefer '-' rather than '+',
> > since we're already using '-' to separate the first three terms
> > that make up the job name - no reason why the variant should be
> > treated differently in the naming scheme.
> 
> I went for '+' in the Ansible part because using '-' would
> introduce ambiguity: is 'libvirt-mingw' a variant of libvirt, or
> just a completely different project like 'libvirt-dbus'?
> 
> In the case of Jenkins jobs, we have already lost that battle
> thanks to jobs like virt-manager-master-build, so I guess it's
> okay to keep using '-' :)
> 
> > > We could then even think about dropping autotools-mingw-job
> > > altogether and instead use generic-build-job there as well,
> > > like
> > > 
> > >   - project:
> > >   name: libvirt
> > >   jobs:
> > > - generic-build-job:
> > > variant: +mingw
> > > command: '{mingw_build}'
> > > 
> > > where 'mingw_build' would be defined globally.
> > 
> > I don't think that's desirable as {mingw_build} ends up duplicating
> > the shell commands defined by the autotools-build-job template.
> > 
> > Instead we shoudl make use of existing parameterization of the
> > existing autotools template
> > 
> >- project:
> >name: libvirt
> >jobs:
> >  - autotools-build-job:
> >  variant: +mingw
> >  local_env: {mingw32_env}
> >  autogen_args: {mingw32_autogen}
> > 
> > With
> > 
> >   mingw32_env: |
> >   export 
> > PKG_CONFIG_LIBDIR="/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/i686-w64-mingw32/sys-root/mingw/share/pkgconfig"
> >   export 
> > PKG_CONFIG_PATH="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig"
> >  \
> >   export PREFIX=$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw
> > 
> >   mingw32_autogen: --host=i686-w64-mingw32
> 
> Yeah, that looks good too.
> 
> It'll lead to having separate -mingw32 and -mingw64 jobs, though:
> personally I don't have a problem with that, just making sure you
> don't either.

I think that is a benefit :-) There's no reason beyond historical
accident to lump them together !

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] [jenkins-ci PATCH v2 3/6] projects: Add libvirt-master-website job

[Andrea Bolognani]

On Tue, 2018-04-17 at 15:02 +0100, Daniel P. Berrangé wrote:
> On Tue, Apr 17, 2018 at 03:57:04PM +0200, Andrea Bolognani wrote:
> > We could go one further and use
> > 
> >   name: '{name}-{branch}-build{variant}'
> > 
> > for the generic-build-job template and
> > 
> >   variant: +website
> > 
> > for this specific job, then change the autotools-mingw-job
> > template to use
> > 
> >   name: '{name}-{branch}-build+mingw'
> > 
> > to align the job names with the project names at the Ansible
> > level.
> 
> Yes, that would be reasonable, though I prefer '-' rather than '+',
> since we're already using '-' to separate the first three terms
> that make up the job name - no reason why the variant should be
> treated differently in the naming scheme.

I went for '+' in the Ansible part because using '-' would
introduce ambiguity: is 'libvirt-mingw' a variant of libvirt, or
just a completely different project like 'libvirt-dbus'?

In the case of Jenkins jobs, we have already lost that battle
thanks to jobs like virt-manager-master-build, so I guess it's
okay to keep using '-' :)

> > We could then even think about dropping autotools-mingw-job
> > altogether and instead use generic-build-job there as well,
> > like
> > 
> >   - project:
> >   name: libvirt
> >   jobs:
> > - generic-build-job:
> > variant: +mingw
> > command: '{mingw_build}'
> > 
> > where 'mingw_build' would be defined globally.
> 
> I don't think that's desirable as {mingw_build} ends up duplicating
> the shell commands defined by the autotools-build-job template.
> 
> Instead we shoudl make use of existing parameterization of the
> existing autotools template
> 
>- project:
>name: libvirt
>jobs:
>  - autotools-build-job:
>  variant: +mingw
>  local_env: {mingw32_env}
>  autogen_args: {mingw32_autogen}
> 
> With
> 
>   mingw32_env: |
>   export 
> PKG_CONFIG_LIBDIR="/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig:/usr/i686-w64-mingw32/sys-root/mingw/share/pkgconfig"
>   export 
> PKG_CONFIG_PATH="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig" \
>   export PREFIX=$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw
> 
>   mingw32_autogen: --host=i686-w64-mingw32

Yeah, that looks good too.

It'll lead to having separate -mingw32 and -mingw64 jobs, though:
personally I don't have a problem with that, just making sure you
don't either.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [dbus PATCH 05/25] Implement MemoryPeek method for Domain Interface

[Ján Tomko]

On Tue, Apr 17, 2018 at 02:04:24PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  8 
src/domain.c| 38 ++
2 files changed, 46 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [dbus PATCH 04/25] Implement BlockPeek metohd for Domain Interface

[Ján Tomko]

On Tue, Apr 17, 2018 at 02:04:23PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Domain.xml |  9 +
src/domain.c| 39 +++
2 files changed, 48 insertions(+)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index 48b8a95..85e2cf6 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -54,6 +54,15 @@
  
  

+
+  https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateWithFlags"/>


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


+  
+  
+  


Even though size_t is (usually) 64 bits, unsigned should be enough given
the documented RPC limits.


+  
+  
+

  https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateWithFlags"/>


With the doc link fixed:

Reviewed-by: Ján Tomko 

Jano


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