Re: [libvirt] [PATCH 3/3] nodedev: update caps before invoking nodedev driver interfaces

[Wuzongyong (Euler Dept)]

It is a bit inefficient about my patch, and it is imperfect.
I wish to see a better idea to fix this problem.

And again, thank you for your correction to my patches.

Thanks,
Zongyong Wu

> -Original Message-
> From: Erik Skultety [mailto:eskul...@redhat.com]
> Sent: Thursday, January 11, 2018 6:16 PM
> To: Wuzongyong (Euler Dept) 
> Cc: libvir-list@redhat.com; weijinfen ; Wanzongshun
> (Vincent) 
> Subject: Re: [PATCH 3/3] nodedev: update caps before invoking nodedev
> driver interfaces
> 
> On Wed, Jan 10, 2018 at 08:14:51PM +0800, Wu Zongyong wrote:
> > Some capabilities of node devices rely on what driver they bound to,
> > and therefore, these capabilities may change when the driver change.
> > So, it is necessary to manually update devices' capabilities each time
> > before nodedev driver interfaces invoked.
> >
> > Signed-off-by: Wu Zongyong 
> > ---
> 
> Thank you for posting the patch, since I hadn't noticed the problem with
> other APIs until I read it.
> It was a sad realization that a driver change is not reflected by a
> udev/kernel CHANGE event, that would make things much much simpler. I have
> an idea to either make this patch shorter or not needed at all though,
> we'll see when I finish my patch (it's a long-needed refactor). Despite I
> haven't found any major flaws in this patch, let's just put it on hold for
> a while until I finish my investigation/work on my patch and see we can
> really do better here.
> 
> Thanks,
> Erik

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

[libvirt] [PATCH] qemu: Fix type of a completed job

[Jiri Denemark]

Libvirt 3.7.0 and earlier libvirt reported a migration job as completed
immediately after QEMU finished sending migration data at which point
migration was not really complete yet. Commit v3.7.0-29-g3f2d6d829e
fixed this, but caused a regression in reporting statistics for
completed jobs which started reporting the job as still running. This
happened because the completed job statistics including the job status
are copied from the running job before we finally mark it as completed.

Let's make sure QEMU_DOMAIN_JOB_STATUS_COMPLETED is always set in the
completed job info even when the job has not finished yet.

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

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

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c1ceb164d1..a56bc596ff 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1646,8 +1646,10 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 qemuDomainJobInfoUpdateTime(jobInfo);
 qemuDomainJobInfoUpdateDowntime(jobInfo);
 VIR_FREE(priv->job.completed);
-if (VIR_ALLOC(priv->job.completed) == 0)
+if (VIR_ALLOC(priv->job.completed) == 0) {
 *priv->job.completed = *jobInfo;
+priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
+}
 
 if (asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT &&
 jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED)
@@ -5479,8 +5481,9 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
 }
 
 if (dom) {
-priv->job.completed = jobInfo;
-jobInfo = NULL;
+VIR_STEAL_PTR(priv->job.completed, jobInfo);
+priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
+
 if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
 QEMU_MIGRATION_COOKIE_STATS) < 0)
 VIR_WARN("Unable to encode migration cookie");
-- 
2.15.1

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

[libvirt] [PATCH] m4: Don't enable bash-completion by default

[Michal Privoznik]

Due to the way that check logic was written we basically enabled
bash completion whenever readline was enabled. This is not right
because it made bash-completion pkg-config module required.

Signed-off-by: Michal Privoznik 
---

Even though this would qualify as a build breaker, I'd rather have
another pair of eyes to look at it (sorry Cyclops).

 m4/virt-bash-completion.m4 | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4
index e1ef58740..c8342176f 100644
--- a/m4/virt-bash-completion.m4
+++ b/m4/virt-bash-completion.m4
@@ -30,14 +30,10 @@ AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [
   AC_REQUIRE([LIBVIRT_CHECK_READLINE])
 
   if test "x$with_readline" != "xyes" ; then
-if test "x$with_bash_completion" != "xyes" ; then
-  with_bash_completion=no
-else
+if test "x$with_bash_completion" = "xyes" ; then
   AC_MSG_ERROR([readline is required for bash completion support])
-fi
-  else
-if test "x$with_bash_completion" = "xcheck" ; then
-  with_bash_completion=yes
+else
+  with_bash_completion=no
 fi
   fi
 
-- 
2.13.6

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

[libvirt] [RFC PATCH] lxc: Up back the veth interfaces by default

[Benjamin Cama]

Upping an interface without configuring it is not a “cardinal sin” but a
sensible way to achieve auto-configuration, e.g. with IPv6 SLAAC (RFC
4862). If NetworkManager has troube with interfaces having only a
link-local address, this is a bug in NetworkManager, not in libvirt; it
should listen for router advertisements to decide if some interface has
global connectivity or not.

With network interfaces up by default, stateless containers can be
easily auto-configured through the network with SLAAC, without any
specific configuration from the host system.

This reverts commit c3cf3c43a0bb2e0e4909c32821e20f607635ec85.

Signed-off-by: Benjamin Cama 
---
Hi,

The patch that I propose to revert basically broke my workflow for light
stateless containers, where they could be auto-configured on IPv6-only network
through SLAAC. Of course, fully-fledged containers can bring up the interface
themselves, but this behavior had previously been the default for quite some
time, and is even indicated as default in src/conf/domain_conf.h ("Default link
state (up)").

I cannot find any real justification for the patch I am reverting, and the
bugzilla looks private so I can not comment on the NetworkManager behavior,
which looks very buggy to me.

Please tell me if you think this is wrong. Also, please Cc me, I am not
subscribed.

Regards.

 .mailmap|  1 +
 src/lxc/lxc_container.c |  7 +--
 src/lxc/lxc_native.c| 10 ++
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/.mailmap b/.mailmap
index 2f0fc901e..9dc3bff85 100644
--- a/.mailmap
+++ b/.mailmap
@@ -36,6 +36,7 @@
  
  
  
+ 
  
  
  
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 96fceaf1b..e546f0aaf 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -517,12 +517,7 @@ lxcContainerRenameAndEnableInterfaces(virDomainDefPtr 
vmDef,
 if (virNetDevSetName(veths[i], newname) < 0)
 goto cleanup;
 
-/* Only enable this device if there is a reason to do so (either
- * at least one IP was specified, or link state was set to up in
- * the config)
- */
-if (netDef->guestIP.nips ||
-netDef->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP) {
+if (netDef->linkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) {
 VIR_DEBUG("Enabling %s", newname);
 if (virNetDevSetOnline(newname, true) < 0)
 goto cleanup;
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index fdc03a57e..f77a2a910 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -349,10 +349,12 @@ lxcCreateNetDef(const char *type,
 if (VIR_ALLOC(net) < 0)
 goto error;
 
-if (STREQ_NULLABLE(flag, "up"))
-net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
-else
-net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN;
+if (flag) {
+if (STREQ(flag, "up"))
+net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
+else
+net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN;
+}
 
 if (VIR_STRDUP(net->ifname_guest, name) < 0)
 goto error;
-- 
2.11.0

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

Re: [libvirt] [PATCH 0/7] Keep non-persistent changes alive in snapshot

[Scott Garfinkle]

On Mon, 2017-10-30 at 14:21 +0530, Kothapally Madhu Pavan wrote:
> Restoring to a snapshot should not overwrite the persistent XML
> configuration
> of a snapshot as a side effect. This patchset fixes the same.
> Currently,
> virDomainSnapshotDef only saves active domain definition of the
> guest.
> And on restore the active domain definition is used as both active
> and
> inactive domain definitions. This will make the non-persistent
> changes
> persistent in snapshot image. This patchset allows to save inactive
> domain
> definition as well and on snapshot-revert non-persistent
> configuration is
> restored as is.
> 
> Currently, snapshot-revert is making non-presistent changes as
> persistent.
> Here are the steps to reproduce.
> Step1: virsh define $dom
> Step2: virsh attach-device $dom $memory-device.xml --live
> Step3: virsh snapshot-create $dom
> Step4: virsh destroy $dom
> Step5: virsh snapshot-revert $dom $snapshot-name
> Step6: virsh destroy $dom
> Step7: virsh start $dom
> Here we still have $memory-device attached in Step2.
> 
> This patchset is attempting to solve this issue. This patchset will
> also
> allow user to dump and edit inactive XML configuration of a snapshot.
> Dumping inactive domain definition of a snapshot is important as
> --redefine uses snapshot-dumpxml output to redefine a snapshot.
> 
> Kothapally Madhu Pavan (7):
>   qemu: Store inactive domain configuration in snapshot
>   qemu: Use active and inactive snapshot configuration on restore
>   conf: Allow editing inactive snapshot configuration
>   virsh: Dump inactive XML configuration of snapshot using
> snapshot-dumpxml
>   virsh: Edit inactive XML configuration of snapshot using snapshot-
> edit
>   virsh: Allow restoring snapshot with non-persistent configuration
>   tests: docs: Add schema and testcase for domainsnapshot
> 
>  docs/schemas/domainsnapshot.rng| 19 +
>  include/libvirt/libvirt-domain-snapshot.h  | 10 ++-
>  include/libvirt/libvirt-domain.h   |  1 +
>  src/conf/domain_conf.c |  6 +-
>  src/conf/domain_conf.h |  2 +
>  src/conf/snapshot_conf.c   | 48
> -
>  src/conf/snapshot_conf.h   |  1 +
>  src/qemu/qemu_driver.c | 33 -
>  .../full_domain_withinactive.xml   | 83
> ++
>  tests/domainsnapshotxml2xmltest.c  |  1 +
>  tools/virsh-snapshot.c | 20 ++
>  tools/virsh.pod| 37 +-
>  12 files changed, 251 insertions(+), 10 deletions(-)
>  create mode 100644
> tests/domainsnapshotxml2xmlout/full_domain_withinactive.xml

Could someone please review the patch set? Jiri mentioned in early
December that it was on his list; perhaps this fell through the cracks
or got displaced by the (admittedly more urgent) Spectre/Meltdown
patches.

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

Re: [libvirt] [PATCH 00/18] Make bash completion great again

[Michal Privoznik]

On 01/11/2018 11:52 AM, Martin Kletzander wrote:
> On Tue, Jan 02, 2018 at 06:11:53PM +0100, Michal Privoznik wrote:
>> Technically, this is a v2 of [1], but this implements the feature from
>> different angle and therefore it's a start of new series.
>>
>>
>> What's implemented?
>> ===
>> Auto completion for both interactive and non-interactive
>> virsh/virt-admin.
>>
>>
>> Known limitations
>> =
>> Currently, just options completers work. I mean, to bring up list
>> of domains you have to:
>>
>>  virsh # start --domain 
>>
>> Doing bare:
>>
>>  virsh # start 
>>
>> brings up list of --options. With the new approach implemented
>> here it should be easy to implement this. But that can be saved
>> for later.
>>
>>
>> How to test this?
>> =
>>
>> Interactive completion should work out of the box. Just make sure
>> you're connected. Completers don't connect! You certainly don't
>> want ssh's 'Password:' prompt show on , do you?
>> Non-interactive completers do connect, but in order to avoid any
>> password prompts, /dev/stdin is closed before anything else is
>> done. In order to test it, just:
>>
>>  libvirt.git $ source tools/bash-completion/vsh
>>
>> Now, bash completion should work:
>>
>>  libvirt.git $ ./tools/virsh -c qemu:///system start --domain 
>>
>>
>> Notes
>> =
>>
>> As usual, you can find all the patches on my github [2]. I've
>> tried to work in all the review suggestions from v1. Due to
>> changes in design (reusing parse code instead of duplicating it)
>> not all suggestions were possible to work in though.
>>
>> Michal
>>
> 
> ACK to all if you fix the two possible NULL problems (04/18 and 08/18).

Thank you guys. I've pushed these. Now all that's left to do is to write
the completer callbacks, but that should be fairly easy now O:-)

Michal

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

Re: [libvirt] [PATCH 08/18 v2] vshCommandOpt: Allow caller avoiding assert()

[John Ferlan]

On 01/11/2018 11:38 AM, Michal Privoznik wrote:
> In the future, completer callbacks will receive partially parsed
> command (and thus possibly incomplete). However, we still want
> them to use command options fetching APIs we already have (e.g.
> vshCommandOpt*()) and at the same time don't report any errors
> (nor call any asserts).
> 
> Signed-off-by: Michal Privoznik 
> ---
>  tools/vsh.c | 24 ++--
>  tools/vsh.h |  3 ++-
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 

Looks good to me...

Reviewed-by: John Ferlan 

Tks-

John

> diff --git a/tools/vsh.c b/tools/vsh.c
> index 54e59fff6..b113c8c95 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
>   * to the option if found, 0 with *OPT set to NULL if the name is
>   * valid and the option is not required, -1 with *OPT set to NULL if
>   * the option is required but not present, and assert if NAME is not
> - * valid (which indicates a programming error).  No error messages are
> - * issued if a value is returned.
> + * valid (which indicates a programming error) unless cmd->skipChecks
> + * is set. No error messages are issued if a value is returned.
>   */
>  static int
>  vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
> @@ -828,15 +828,19 @@ vshCommandOpt(const vshCmd *cmd, const char *name, 
> vshCmdOpt **opt,
>  
>  /* See if option is valid and/or required.  */
>  *opt = NULL;
> -while (valid) {
> -assert(valid->name);
> -if (STREQ(name, valid->name))
> -break;
> -valid++;
> +
> +if (!cmd->skipChecks) {
> +while (valid && valid->name) {
> +if (STREQ(name, valid->name))
> +break;
> +valid++;
> +}
> +
> +assert(valid && (!needData || valid->type != VSH_OT_BOOL));
> +
> +if (valid->flags & VSH_OFLAG_REQ)
> +ret = -1;
>  }
> -assert(!needData || valid->type != VSH_OT_BOOL);
> -if (valid->flags & VSH_OFLAG_REQ)
> -ret = -1;
>  
>  /* See if option is present on command line.  */
>  while (candidate) {
> diff --git a/tools/vsh.h b/tools/vsh.h
> index 8f7df9ff8..112b1b57d 100644
> --- a/tools/vsh.h
> +++ b/tools/vsh.h
> @@ -188,7 +188,8 @@ struct _vshCmdDef {
>  struct _vshCmd {
>  const vshCmdDef *def;   /* command definition */
>  vshCmdOpt *opts;/* list of command arguments */
> -vshCmd *next;  /* next command */
> +vshCmd *next;   /* next command */
> +bool skipChecks;/* skip validity checks when retrieving opts 
> */
>  };
>  
>  /*
> 

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

Re: [libvirt] [PATCH python 7/7] Require libvirt native version matching py version by default

[Daniel P. Berrange]

On Thu, Jan 11, 2018 at 06:00:34PM +0100, Jiri Denemark wrote:
> On Thu, Jan 11, 2018 at 16:43:39 +, Daniel P. Berrange wrote:
> > Although we're capable of building against any libvirt >= 0.9.11, 99% of the
> > time we want RPM builds to be done against matching libvirt version, 
> > otherwise
> > we might silently build against an unexpected/wrong version.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  libvirt-python.spec.in | 2 +-
> >  setup.py   | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
> > index 0087c78..6afa6f8 100644
> > --- a/libvirt-python.spec.in
> > +++ b/libvirt-python.spec.in
> > @@ -35,7 +35,7 @@ Source0: 
> > http://libvirt.org/sources/python/%{name}-%{version}.tar.gz
> >  Url: http://libvirt.org
> >  License: LGPLv2+
> >  Group: Development/Libraries
> > -BuildRequires: libvirt-devel >= @C_VERSION@
> > +BuildRequires: libvirt-devel >= %{version}
> 
> Shouldn't we even restrict this to ==? If libvirt-python is built
> against newer version of libvirt, its generator may provide incorrect
> implementation for new APIs which need to be implemented manually. And
> only new enough libvirt-python will have these new APIs in the override
> list.

Hmm, yes, that's a good point. We don't officially support building old
python against newer libvirt.  We probably ought to validate that in the
generator too rather than leave the user to get bizarre error messages
or badly generated code.

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] qemuDomainRemoveMemoryDevice: unlink() memory backing file

[Zack Cornelius]

- Original Message -
> From: "Michal Privoznik" 
> To: libvir-list@redhat.com
> Cc: "Zack Cornelius" 
> Sent: Thursday, January 11, 2018 6:24:57 AM
> Subject: [PATCH] qemuDomainRemoveMemoryDevice: unlink() memory backing file

>
> Zack, can you please check if this patch is suitable for your use cases?
> 

This patch will work for Kove's use cases.

--Zack

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

Re: [libvirt] [PATCH python 7/7] Require libvirt native version matching py version by default

[Jiri Denemark]

On Thu, Jan 11, 2018 at 16:43:39 +, Daniel P. Berrange wrote:
> Although we're capable of building against any libvirt >= 0.9.11, 99% of the
> time we want RPM builds to be done against matching libvirt version, otherwise
> we might silently build against an unexpected/wrong version.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-python.spec.in | 2 +-
>  setup.py   | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
> index 0087c78..6afa6f8 100644
> --- a/libvirt-python.spec.in
> +++ b/libvirt-python.spec.in
> @@ -35,7 +35,7 @@ Source0: 
> http://libvirt.org/sources/python/%{name}-%{version}.tar.gz
>  Url: http://libvirt.org
>  License: LGPLv2+
>  Group: Development/Libraries
> -BuildRequires: libvirt-devel >= @C_VERSION@
> +BuildRequires: libvirt-devel >= %{version}

Shouldn't we even restrict this to ==? If libvirt-python is built
against newer version of libvirt, its generator may provide incorrect
implementation for new APIs which need to be implemented manually. And
only new enough libvirt-python will have these new APIs in the override
list.

Jirka

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

Re: [libvirt] [PATCH python 5/7] Adapt to rename of py2 RPMs from python- to python2- prefix

[Daniel P. Berrange]

On Thu, Jan 11, 2018 at 04:43:37PM +, Daniel P. Berrange wrote:
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-python.spec.in | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
> index b667ebe..cc5a5a5 100644
> --- a/libvirt-python.spec.in
> +++ b/libvirt-python.spec.in
> @@ -18,6 +18,12 @@
>  %define _with_python3 1
>  %endif
>  
> +# Whether py2 packages are assumed to have python2- name prefix
> +%define py2_versioned_deps 0
> +%if 0%{?fedora} || 0%{?rhel} > 7
> +%define py2_versioned_deps 1
> +%endif
> +
>  %{!?with_python2: %define with_python2 %{_with_python2}}
>  %{!?with_python3: %define with_python3 %{_with_python3}}
>  
> @@ -31,10 +37,16 @@ License: LGPLv2+
>  Group: Development/Libraries
>  BuildRequires: libvirt-devel >= @C_VERSION@
>  %if %{with_python2}
> +%if %{py2_versioned_deps}
> +BuildRequires: python2-devel
> +BuildRequires: pyth2on2-nose

Yes, I will remove this intentional typo i added during testing :-)

> +BuildRequires: python2-lxml
> +%else
>  BuildRequires: python-devel
>  BuildRequires: python-nose
>  BuildRequires: python-lxml
>  %endif
> +%endif
>  %if %{with_python3}
>  BuildRequires: python3-devel
>  BuildRequires: python3-nose
> -- 
> 2.14.3
> 

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 python 5/7] Adapt to rename of py2 RPMs from python- to python2- prefix

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 12 
 1 file changed, 12 insertions(+)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index b667ebe..cc5a5a5 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -18,6 +18,12 @@
 %define _with_python3 1
 %endif
 
+# Whether py2 packages are assumed to have python2- name prefix
+%define py2_versioned_deps 0
+%if 0%{?fedora} || 0%{?rhel} > 7
+%define py2_versioned_deps 1
+%endif
+
 %{!?with_python2: %define with_python2 %{_with_python2}}
 %{!?with_python3: %define with_python3 %{_with_python3}}
 
@@ -31,10 +37,16 @@ License: LGPLv2+
 Group: Development/Libraries
 BuildRequires: libvirt-devel >= @C_VERSION@
 %if %{with_python2}
+%if %{py2_versioned_deps}
+BuildRequires: python2-devel
+BuildRequires: pyth2on2-nose
+BuildRequires: python2-lxml
+%else
 BuildRequires: python-devel
 BuildRequires: python-nose
 BuildRequires: python-lxml
 %endif
+%endif
 %if %{with_python3}
 BuildRequires: python3-devel
 BuildRequires: python3-nose
-- 
2.14.3

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

[libvirt] [PATCH python 7/7] Require libvirt native version matching py version by default

[Daniel P. Berrange]

Although we're capable of building against any libvirt >= 0.9.11, 99% of the
time we want RPM builds to be done against matching libvirt version, otherwise
we might silently build against an unexpected/wrong version.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 2 +-
 setup.py   | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 0087c78..6afa6f8 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -35,7 +35,7 @@ Source0: 
http://libvirt.org/sources/python/%{name}-%{version}.tar.gz
 Url: http://libvirt.org
 License: LGPLv2+
 Group: Development/Libraries
-BuildRequires: libvirt-devel >= @C_VERSION@
+BuildRequires: libvirt-devel >= %{version}
 %if %{with_python2}
 %if %{py2_versioned_deps}
 BuildRequires: python2-devel
diff --git a/setup.py b/setup.py
index 85af965..5e29c8a 100755
--- a/setup.py
+++ b/setup.py
@@ -175,8 +175,7 @@ class my_sdist(sdist):
 f2 = open('libvirt-python.spec', 'w')
 for line in f1:
 f2.write(line
- .replace('@PY_VERSION@', self.distribution.get_version())
- .replace('@C_VERSION@', MIN_LIBVIRT))
+ .replace('@PY_VERSION@', self.distribution.get_version()))
 f1.close()
 f2.close()
 
-- 
2.14.3

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

Re: [libvirt] [PATCH] rpm: updates wrt min required fedora version

[Jiri Denemark]

On Thu, Jan 11, 2018 at 16:31:45 +, Daniel P. Berrange wrote:
> Update the min fedora to 25. Use a macro to record the min versions so that 
> the
> later error message is always in sync with the earlier version check. Clarify
> the comment that refers to guessing of dist which does not actually happen.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt.spec.in | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Jiri Denemark 

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

[libvirt] [PATCH python 6/7] Turn on python3 sub-RPMs for RHEL > 7

[Daniel P. Berrange]

It is expected that future RHEL-8 will have python3 by default, so enable that.
It is unclear whether python2 will still be available, so leave that enabled
for now.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index cc5a5a5..0087c78 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -14,7 +14,7 @@
 
 %define _with_python2 1
 %define _with_python3 0
-%if 0%{?fedora}
+%if 0%{?fedora} || 0%{?rhel} > 7
 %define _with_python3 1
 %endif
 
-- 
2.14.3

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

[libvirt] [PATCH python 4/7] Add emacs mode marker to activate rpm-spec highlighting

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index a98b902..b667ebe 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -1,3 +1,5 @@
+# -*- rpm-spec -*-
+
 # This spec file assumes you are building on a Fedora or RHEL version
 # that's still supported by the vendor. It may work on other distros
 # or versions, but no effort will be made to ensure that going forward
-- 
2.14.3

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

[libvirt] [PATCH python 1/7] Allow disabling of python2 RPM build

[Daniel P. Berrange]

With Fedora modularity, it is possible to have add-on repos for multiple
versions of python3. It is thus desirable to be able to build libvirt-python
in these repos, with only the python3 sub-RPMs enabled.

Thus also helps if future RHEL/Fedora drop python2 entirely from their default
repos.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 13 +
 1 file changed, 13 insertions(+)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 4d0262d..5bbebeb 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -1,4 +1,5 @@
 
+%define with_python2 1
 %define with_python3 0
 %if 0%{?fedora}
 %define with_python3 1
@@ -13,9 +14,11 @@ Url: http://libvirt.org
 License: LGPLv2+
 Group: Development/Libraries
 BuildRequires: libvirt-devel >= @C_VERSION@
+%if %{with_python2}
 BuildRequires: python-devel
 BuildRequires: python-nose
 BuildRequires: python-lxml
+%endif
 %if %{with_python3}
 BuildRequires: python3-devel
 BuildRequires: python3-nose
@@ -32,6 +35,7 @@ written in the Python programming language to use the 
interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
 
+%if %{with_python2}
 %package -n python2-libvirt
 Summary: The libvirt virtualization API python2 binding
 Url: http://libvirt.org
@@ -46,6 +50,7 @@ The python2-libvirt package contains a module that permits 
applications
 written in the Python programming language to use the interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
+%endif
 
 %if %{with_python3}
 %package -n python3-libvirt
@@ -73,23 +78,30 @@ of recent versions of Linux (and other OSes).
 find examples -type f -exec chmod 0644 \{\} \;
 
 %build
+%if %{with_python2}
 CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build
+%endif
 %if %{with_python3}
 CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %endif
 
 %install
+%if %{with_python2}
 %{__python} setup.py install --skip-build --root=%{buildroot}
+%endif
 %if %{with_python3}
 %{__python3} setup.py install --skip-build --root=%{buildroot}
 %endif
 
 %check
+%if %{with_python2}
 %{__python} setup.py test
+%endif
 %if %{with_python3}
 %{__python3} setup.py test
 %endif
 
+%if %{with_python2}
 %files -n python2-libvirt
 %defattr(-,root,root)
 %doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/
@@ -98,6 +110,7 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{_libdir}/python2*/site-packages/libvirt_lxc.py*
 %{_libdir}/python2*/site-packages/libvirtmod*
 %{_libdir}/python2*/site-packages/*egg-info
+%endif
 
 %if %{with_python3}
 %files -n python3-libvirt
-- 
2.14.3

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

[libvirt] [PATCH python 3/7] Add checks for min supported distros

[Daniel P. Berrange]

Be clear about which distros we aim to support with the specfile, so we know
what we can cleanup in the spec later.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 16 
 1 file changed, 16 insertions(+)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 1619e26..a98b902 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -1,3 +1,14 @@
+# This spec file assumes you are building on a Fedora or RHEL version
+# that's still supported by the vendor. It may work on other distros
+# or versions, but no effort will be made to ensure that going forward
+%define min_rhel 6
+%define min_fedora 25
+
+%if (0%{?fedora} && 0%{?fedora} >= %{min_fedora}) || (0%{?rhel} && 0%{?rhel} 
>= %{min_rhel})
+%define supported_platform 1
+%else
+%define supported_platform 0
+%endif
 
 %define _with_python2 1
 %define _with_python3 0
@@ -81,6 +92,11 @@ of recent versions of Linux (and other OSes).
 find examples -type f -exec chmod 0644 \{\} \;
 
 %build
+%if ! %{supported_platform}
+echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}"
+exit 1
+%endif
+
 %if %{with_python2}
 CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build
 %endif
-- 
2.14.3

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

[libvirt] [PATCH python 2/7] Allow override of which sub-RPMs to build

[Daniel P. Berrange]

Allow using

  rpmbuild --define "with_python2 0"

to override the default logic about which python sub-RPMs to build

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 5bbebeb..1619e26 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -1,10 +1,13 @@
 
-%define with_python2 1
-%define with_python3 0
+%define _with_python2 1
+%define _with_python3 0
 %if 0%{?fedora}
-%define with_python3 1
+%define _with_python3 1
 %endif
 
+%{!?with_python2: %define with_python2 %{_with_python2}}
+%{!?with_python3: %define with_python3 %{_with_python3}}
+
 Summary: The libvirt virtualization API python2 binding
 Name: libvirt-python
 Version: @PY_VERSION@
-- 
2.14.3

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

[libvirt] [PATCH python 0/7] Misc improvements to RPM spec

[Daniel P. Berrange]

Various improvements to the RPM spec to help future Fedora/RHEL maint

Daniel P. Berrange (7):
  Allow disabling of python2 RPM build
  Allow override of which sub-RPMs to build
  Add checks for min supported distros
  Add emacs mode marker to activate rpm-spec highlighting
  Adapt to rename of py2 RPMs from python- to python2- prefix
  Turn on python3 sub-RPMs for RHEL > 7
  Require libvirt native version matching py version by default

 libvirt-python.spec.in | 54 ++
 setup.py   |  3 +--
 2 files changed, 51 insertions(+), 6 deletions(-)

-- 
2.14.3

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

[libvirt] [PATCH 08/18 v2] vshCommandOpt: Allow caller avoiding assert()

[Michal Privoznik]

In the future, completer callbacks will receive partially parsed
command (and thus possibly incomplete). However, we still want
them to use command options fetching APIs we already have (e.g.
vshCommandOpt*()) and at the same time don't report any errors
(nor call any asserts).

Signed-off-by: Michal Privoznik 
---
 tools/vsh.c | 24 ++--
 tools/vsh.h |  3 ++-
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 54e59fff6..b113c8c95 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
  * to the option if found, 0 with *OPT set to NULL if the name is
  * valid and the option is not required, -1 with *OPT set to NULL if
  * the option is required but not present, and assert if NAME is not
- * valid (which indicates a programming error).  No error messages are
- * issued if a value is returned.
+ * valid (which indicates a programming error) unless cmd->skipChecks
+ * is set. No error messages are issued if a value is returned.
  */
 static int
 vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
@@ -828,15 +828,19 @@ vshCommandOpt(const vshCmd *cmd, const char *name, 
vshCmdOpt **opt,
 
 /* See if option is valid and/or required.  */
 *opt = NULL;
-while (valid) {
-assert(valid->name);
-if (STREQ(name, valid->name))
-break;
-valid++;
+
+if (!cmd->skipChecks) {
+while (valid && valid->name) {
+if (STREQ(name, valid->name))
+break;
+valid++;
+}
+
+assert(valid && (!needData || valid->type != VSH_OT_BOOL));
+
+if (valid->flags & VSH_OFLAG_REQ)
+ret = -1;
 }
-assert(!needData || valid->type != VSH_OT_BOOL);
-if (valid->flags & VSH_OFLAG_REQ)
-ret = -1;
 
 /* See if option is present on command line.  */
 while (candidate) {
diff --git a/tools/vsh.h b/tools/vsh.h
index 8f7df9ff8..112b1b57d 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -188,7 +188,8 @@ struct _vshCmdDef {
 struct _vshCmd {
 const vshCmdDef *def;   /* command definition */
 vshCmdOpt *opts;/* list of command arguments */
-vshCmd *next;  /* next command */
+vshCmd *next;   /* next command */
+bool skipChecks;/* skip validity checks when retrieving opts */
 };
 
 /*
-- 
2.13.6

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

Re: [libvirt] [PATCH v2] treat host models as case-insensitive strings

[Scott Garfinkle]

On Thu, 2018-01-11 at 11:19 -0500, John Ferlan wrote:
> 
> On 12/26/2017 02:55 PM, Scott Garfinkle wrote:
> > Qemu now allows case-insensitive specification of CPU models. This
> > fixes the resulting problems on (at least) POWER arch machines.
> 
> Would have been great to reference which qemu commit number, but
> there's probably way too many.  Perhaps best to note it's as of QEMU
> 2.11.  My quick search turns up "03c9141d75" and "4a12c699d", is that
> about right?
Thanks, next time I'll try to be more specific. Yes Qemu 2.11. The full
patch series is at
   https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04651.html

> > 
> > Patch V2: Change only the internal interface. This solves the
> > actual problem at
> > hand of reporting unsupported models now that qemu allows case-
> > insensitive
> > strings (e.g. "Power8" instead of "POWER8").
> 
> This hunk would typically go under the "---" below to give a hint to
> the reviewer about what changed.
ack

> Hopefully no one someday decides POWER8 is more "powerful" than
> Power8 ;-) (couldn't resist).
groan  :-)

> > 
> > Signed-off-by: Scott Garfinkle 
> > ---
> >  src/conf/domain_capabilities.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> 
> This seems like a reasonable way to approach the problem.  I can fix
> up the comment message a bit and will push later... Just want to make
> sure that no one else comes up with a latent concern...  The only
> thing that springs to my mind is migration possibly.
It seems to me that the in the worst we might be more permissive with
the patch than without; I am hopeful for the sake of sanity that nobody
actually has two model names disambiguated only by case!

> Reviewed-by: John Ferlan 
> 
> John
> 
> > diff --git a/src/conf/domain_capabilities.c
> > b/src/conf/domain_capabilities.c
> > index e7323a8..f7d9be5 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -271,7 +271,7 @@
> > virDomainCapsCPUModelsGet(virDomainCapsCPUModelsPtr cpuModels,
> >  return NULL;
> >  
> >  for (i = 0; i < cpuModels->nmodels; i++) {
> > -if (STREQ(cpuModels->models[i].name, name))
> > +if (STRCASEEQ(cpuModels->models[i].name, name))
> >  return cpuModels->models + i;
> >  }
> >  
> > 
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

[libvirt] [PATCH] rpm: updates wrt min required fedora version

[Daniel P. Berrange]

Update the min fedora to 25. Use a macro to record the min versions so that the
later error message is always in sync with the earlier version check. Clarify
the comment that refers to guessing of dist which does not actually happen.

Signed-off-by: Daniel P. Berrange 
---
 libvirt.spec.in | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 7e1b6a27d3..713961573a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1,10 +1,12 @@
 # -*- rpm-spec -*-
 
 # This spec file assumes you are building on a Fedora or RHEL version
-# that's still supported by the vendor: that means Fedora 23 or newer,
-# or RHEL 6 or newer. It may need some tweaks for other distros.
-# If neither fedora nor rhel was defined, try to guess them from dist
-%if (0%{?fedora} && 0%{?fedora} >= 23) || (0%{?rhel} && 0%{?rhel} >= 6)
+# that's still supported by the vendor. It may work on other distros
+# or versions, but no effort will be made to ensure that going forward.
+%define min_rhel 6
+%define min_fedora 25
+
+%if (0%{?fedora} && 0%{?fedora} >= %{min_fedora}) || (0%{?rhel} && 0%{?rhel} 
>= %{min_rhel})
 %define supported_platform 1
 %else
 %define supported_platform 0
@@ -1132,7 +1134,7 @@ rm -rf .git
 
 %build
 %if ! %{supported_platform}
-echo "This RPM requires either Fedora >= 20 or RHEL >= 6"
+echo "This RPM requires either Fedora >= %{min_fedora} or RHEL >= %{min_rhel}"
 exit 1
 %endif
 
-- 
2.14.3

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

Re: [libvirt] [PATCH v2] treat host models as case-insensitive strings

[John Ferlan]

On 12/26/2017 02:55 PM, Scott Garfinkle wrote:
> Qemu now allows case-insensitive specification of CPU models. This fixes the
> resulting problems on (at least) POWER arch machines.

Would have been great to reference which qemu commit number, but there's
probably way too many.  Perhaps best to note it's as of QEMU 2.11.  My
quick search turns up "03c9141d75" and "4a12c699d", is that about right?
Although there's quite a few other ones too

> 
> Patch V2: Change only the internal interface. This solves the actual problem 
> at
> hand of reporting unsupported models now that qemu allows case-insensitive
> strings (e.g. "Power8" instead of "POWER8").

This hunk would typically go under the "---" below to give a hint to the
reviewer about what changed.

Hopefully no one someday decides POWER8 is more "powerful" than Power8
;-) (couldn't resist).

> 
> Signed-off-by: Scott Garfinkle 
> ---
>  src/conf/domain_capabilities.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

This seems like a reasonable way to approach the problem.  I can fix up
the comment message a bit and will push later... Just want to make sure
that no one else comes up with a latent concern...  The only thing that
springs to my mind is migration possibly.

Reviewed-by: John Ferlan 

John

> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index e7323a8..f7d9be5 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -271,7 +271,7 @@ virDomainCapsCPUModelsGet(virDomainCapsCPUModelsPtr 
> cpuModels,
>  return NULL;
>  
>  for (i = 0; i < cpuModels->nmodels; i++) {
> -if (STREQ(cpuModels->models[i].name, name))
> +if (STRCASEEQ(cpuModels->models[i].name, name))
>  return cpuModels->models + i;
>  }
>  
> 

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

Re: [libvirt] [PATCH] vsh: add a necessary assertion

[Marc Hartmayer]

On Thu, Jan 11, 2018 at 04:18 PM +0100, John Ferlan  wrote:
> On 01/11/2018 10:02 AM, Marc Hartmayer wrote:
>> On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan  
>> wrote:
>>> On 12/18/2017 07:33 AM, Marc Hartmayer wrote:
 This fixes the compilation error (compiled with the compiler option
 '-03').

 In file included from ../../tools/vsh.c:28:0:
 ../../tools/vsh.c: In function 'vshCommandOptStringQuiet':
 ../../tools/vsh.c:838:30: error: potential null pointer dereference 
 [-Werror=null-dereference]
  assert(!needData || valid->type != VSH_OT_BOOL);

 Signed-off-by: Marc Hartmayer 
 Reviewed-by: Boris Fiuczynski 
 ---
  tools/vsh.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/tools/vsh.c b/tools/vsh.c
 index e878119b988f..677eb9db3e41 100644
 --- a/tools/vsh.c
 +++ b/tools/vsh.c
 @@ -816,8 +816,8 @@ vshCommandFree(vshCmd *cmd)
   * to the option if found, 0 with *OPT set to NULL if the name is
   * valid and the option is not required, -1 with *OPT set to NULL if
   * the option is required but not present, and assert if NAME is not
 - * valid (which indicates a programming error).  No error messages are
 - * issued if a value is returned.
 + * valid or the option was not found (which indicates a programming
 + * error).  No error messages are issued if a value is returned.
   */
  static int
  vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
 @@ -835,6 +835,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name, 
 vshCmdOpt **opt,
  break;
  valid++;
  }
 +assert(valid);
 +
>>>
>>> Could we combine the subsequent one to have :
>>>
>>> assert(valid && (!needData || valid->type != VSH_OT_BOOL));
>> 
>> Shall I resend a v2? Thanks.
>> 
>
> Let's see what Michal comes up with in his vsh series, see:
>
> https://www.redhat.com/archives/libvir-list/2018-January/msg00365.html
>
> I had seen his series, so I hesitated in pushing this until I (or
> someone else) had a chance to look at his series and determine if he
> addressed it at all.
>
> It's not like the assert issue is recent, so hopefully we can wait to
> see what he comes up with.

Sure :)

>
> John
>
>>>
>>> ?
>>>
>>> John
>>>
  assert(!needData || valid->type != VSH_OT_BOOL);
  if (valid->flags & VSH_OFLAG_REQ)
  ret = -1;

>>>
>
-- 
Beste Grüße / Kind regards
   Marc Hartmayer

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


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

[libvirt] [PATCH] docs: Add missing encryption type

[John Ferlan]

Update the text to include "luks" as a possible value.

Signed-off-by: John Ferlan 
---
 Pushed as trivial.

 docs/formatstorageencryption.html.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index c4b209527..23efbf932 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -17,7 +17,8 @@
   The top-level tag of volume encryption specification
   is encryption, with a mandatory
   attribute format.  Currently defined values
-  of format are default and qcow.
+  of format are default, qcow,
+  and luks.
   Each value of format implies some expectations about the
   content of the encryption tag.  Other format values may be
   defined in the future.
-- 
2.13.6

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

[libvirt] [PATCH] fixed bug:if expand thread pool, will lose some one

[Di Wei]

---
 src/util/virthreadpool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c
index 10f2bd2..0983ee2 100644
--- a/src/util/virthreadpool.c
+++ b/src/util/virthreadpool.c
@@ -186,6 +186,7 @@ virThreadPoolExpand(virThreadPoolPtr pool, size_t gain, 
bool priority)
 size_t *curWorkers = priority ? >nPrioWorkers : >nWorkers;
 size_t i = 0;
 struct virThreadPoolWorkerData *data = NULL;
+size_t oldNWorkers = *curWorkers;
 
 if (VIR_EXPAND_N(*workers, *curWorkers, gain) < 0)
 return -1;
@@ -198,7 +199,7 @@ virThreadPoolExpand(virThreadPoolPtr pool, size_t gain, 
bool priority)
 data->cond = priority ? >prioCond : >cond;
 data->priority = priority;
 
-if (virThreadCreateFull(&(*workers)[i],
+if (virThreadCreateFull(&(*workers)[i + oldNWorkers],
 false,
 virThreadPoolWorker,
 pool->jobFuncName,
-- 
2.9.3


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

Re: [libvirt] [PATCH] vsh: add a necessary assertion

[John Ferlan]

On 01/11/2018 10:02 AM, Marc Hartmayer wrote:
> On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan  
> wrote:
>> On 12/18/2017 07:33 AM, Marc Hartmayer wrote:
>>> This fixes the compilation error (compiled with the compiler option
>>> '-03').
>>>
>>> In file included from ../../tools/vsh.c:28:0:
>>> ../../tools/vsh.c: In function 'vshCommandOptStringQuiet':
>>> ../../tools/vsh.c:838:30: error: potential null pointer dereference 
>>> [-Werror=null-dereference]
>>>  assert(!needData || valid->type != VSH_OT_BOOL);
>>>
>>> Signed-off-by: Marc Hartmayer 
>>> Reviewed-by: Boris Fiuczynski 
>>> ---
>>>  tools/vsh.c | 6 --
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/vsh.c b/tools/vsh.c
>>> index e878119b988f..677eb9db3e41 100644
>>> --- a/tools/vsh.c
>>> +++ b/tools/vsh.c
>>> @@ -816,8 +816,8 @@ vshCommandFree(vshCmd *cmd)
>>>   * to the option if found, 0 with *OPT set to NULL if the name is
>>>   * valid and the option is not required, -1 with *OPT set to NULL if
>>>   * the option is required but not present, and assert if NAME is not
>>> - * valid (which indicates a programming error).  No error messages are
>>> - * issued if a value is returned.
>>> + * valid or the option was not found (which indicates a programming
>>> + * error).  No error messages are issued if a value is returned.
>>>   */
>>>  static int
>>>  vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>>> @@ -835,6 +835,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name, 
>>> vshCmdOpt **opt,
>>>  break;
>>>  valid++;
>>>  }
>>> +assert(valid);
>>> +
>>
>> Could we combine the subsequent one to have :
>>
>> assert(valid && (!needData || valid->type != VSH_OT_BOOL));
> 
> Shall I resend a v2? Thanks.
> 

Let's see what Michal comes up with in his vsh series, see:

https://www.redhat.com/archives/libvir-list/2018-January/msg00365.html

I had seen his series, so I hesitated in pushing this until I (or
someone else) had a chance to look at his series and determine if he
addressed it at all.

It's not like the assert issue is recent, so hopefully we can wait to
see what he comes up with.

John

>>
>> ?
>>
>> John
>>
>>>  assert(!needData || valid->type != VSH_OT_BOOL);
>>>  if (valid->flags & VSH_OFLAG_REQ)
>>>  ret = -1;
>>>
>>

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

Re: [libvirt] [PATCH 08/18] vshCommandOpt: Allow caller avoiding assert()

[Michal Privoznik]

On 01/11/2018 02:31 PM, Martin Kletzander wrote:
> On Thu, Jan 11, 2018 at 08:09:25AM -0500, John Ferlan wrote:
>>
>>
>> On 01/11/2018 07:55 AM, Martin Kletzander wrote:
>>> On Thu, Jan 11, 2018 at 06:02:58AM -0500, John Ferlan wrote:


 On 01/11/2018 05:50 AM, Martin Kletzander wrote:
> On Tue, Jan 02, 2018 at 06:12:01PM +0100, Michal Privoznik wrote:
>> In the future, completer callbacks will receive partially parsed
>> command (and thus possibly incomplete). However, we still want
>> them to use command options fetching APIs we already have (e.g.
>> vshCommandOpt*()) and at the same time don't report any errors
>> (nor call any asserts).
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>> tools/vsh.c | 7 ---
>> tools/vsh.h | 3 ++-
>> 2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index ebc8d9cb1..d27acb95b 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
>>  * to the option if found, 0 with *OPT set to NULL if the name is
>>  * valid and the option is not required, -1 with *OPT set to NULL if
>>  * the option is required but not present, and assert if NAME is not
>> - * valid (which indicates a programming error).  No error
>> messages are
>> - * issued if a value is returned.
>> + * valid (which indicates a programming error) unless
>> cmd->skipChecks
>> + * is set. No error messages are issued if a value is returned.
>>  */
>> static int
>> vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>> @@ -829,7 +829,8 @@ vshCommandOpt(const vshCmd *cmd, const char
>> *name,
>> vshCmdOpt **opt,
>> /* See if option is valid and/or required.  */
>> *opt = NULL;
>> while (valid) {
>> -assert(valid->name);
>> +if (!cmd->skipChecks)
>> +assert(valid->name);
>
> This can segfault when cmd->skipChecks == False && valid->name ==
> NULL,
> which is what the assert() guarded before.
>
> So either STREQ_NULLABLE or another if.
>

 Hmmm... Also see:

 https://www.redhat.com/archives/libvir-list/2017-December/msg00605.html

 it's related somewhat...

>>>
>>> I don't see how, this is all wrapped in `while (valid)`
>>
>> The other patch is "after" the loop.
>>
>> Look at the entire context... although we know it's a software
>> engineering error to not have some sort of match, some compiler believes
>> we can exit the "while (valid)" loop with "valid == NULL", follwed by
>> the next assert which dereferences @valid without asserting if valid is
>> non-NULL.
>>
> 
> Oh, I guess that patch from Marc should be pushed (I still didn't get to
> s many patches on the list) and this should then handle addition to
> that change as well.  I got confused by your vague "somewhat related" =)

I don't think it should be pushed. The whole loop is foobared. I'm
looking into it. The problem is, we initialize an array of opts like this:

static const vshCmdOptDef opts_iothreadpin[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "iothread",
 .type = VSH_OT_INT,
 .flags = VSH_OFLAG_REQ,
 .help = N_("IOThread ID number")
},
{.name = "cpulist",
 .type = VSH_OT_DATA,
 .flags = VSH_OFLAG_REQ,
 .help = N_("host cpu number(s) to set")
},
VIRSH_COMMON_OPT_DOMAIN_CONFIG,
VIRSH_COMMON_OPT_DOMAIN_LIVE,
VIRSH_COMMON_OPT_DOMAIN_CURRENT,
{.name = NULL}
};


So the array is NOT NULL terminated. Therefore, if we iterate over it
like this:

while (valid) {
...
valid++;
}

we will definitely read outside of the array. But as I've said, I'm
looking into this and I think I have a patch ready. But before that,
please don't push that patch of Marc's as it's not fixing the real
issue.

Michal

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

Re: [libvirt] [PATCH] vsh: add a necessary assertion

[Marc Hartmayer]

On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan  wrote:
> On 12/18/2017 07:33 AM, Marc Hartmayer wrote:
>> This fixes the compilation error (compiled with the compiler option
>> '-03').
>> 
>> In file included from ../../tools/vsh.c:28:0:
>> ../../tools/vsh.c: In function 'vshCommandOptStringQuiet':
>> ../../tools/vsh.c:838:30: error: potential null pointer dereference 
>> [-Werror=null-dereference]
>>  assert(!needData || valid->type != VSH_OT_BOOL);
>> 
>> Signed-off-by: Marc Hartmayer 
>> Reviewed-by: Boris Fiuczynski 
>> ---
>>  tools/vsh.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index e878119b988f..677eb9db3e41 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -816,8 +816,8 @@ vshCommandFree(vshCmd *cmd)
>>   * to the option if found, 0 with *OPT set to NULL if the name is
>>   * valid and the option is not required, -1 with *OPT set to NULL if
>>   * the option is required but not present, and assert if NAME is not
>> - * valid (which indicates a programming error).  No error messages are
>> - * issued if a value is returned.
>> + * valid or the option was not found (which indicates a programming
>> + * error).  No error messages are issued if a value is returned.
>>   */
>>  static int
>>  vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>> @@ -835,6 +835,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name, 
>> vshCmdOpt **opt,
>>  break;
>>  valid++;
>>  }
>> +assert(valid);
>> +
>
> Could we combine the subsequent one to have :
>
> assert(valid && (!needData || valid->type != VSH_OT_BOOL));

Shall I resend a v2? Thanks.

>
> ?
>
> John
>
>>  assert(!needData || valid->type != VSH_OT_BOOL);
>>  if (valid->flags & VSH_OFLAG_REQ)
>>  ret = -1;
>> 
>
-- 
Beste Grüße / Kind regards
   Marc Hartmayer

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


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

Re: [libvirt] [PATCH 04/18] vshCommandStringParse: Allow retrieving partial result

[Michal Privoznik]

On 01/11/2018 11:48 AM, Martin Kletzander wrote:
> On Tue, Jan 02, 2018 at 06:11:57PM +0100, Michal Privoznik wrote:
>> In the future, this function is going to be called from
>> vshReadlineParse() to provide parsed input for completer
>> callbacks. The idea is to allow the callbacks to provide more
>> specific data. For instance, for the following input:
>>
>>  virsh # domifaddr --domain fedora --interface 
>>
>> the --interface completer callback is going to be called. Now, it
>> is more user friendly if the completer offers only those
>> interfaces found in 'fedora' domain. But in order to do that it
>> needs to be able to retrieve partially parsed result.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>> tools/virsh.c  |   4 +-
>> tools/virt-admin.c |   4 +-
>> tools/vsh.c    | 111
>> +
>> tools/vsh.h    |   2 +-
>> 4 files changed, 82 insertions(+), 39 deletions(-)
>>
>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index 2366b7b71..34eb592ef 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -1519,11 +1544,12 @@ vshCommandParse(vshControl *ctl,
>> vshCommandParser *parser)
>>     last->next = arg;
>>     last = arg;
>>
>> -    vshDebug(ctl, VSH_ERR_INFO, "%s: %s(%s): %s\n",
>> - cmd->name,
>> - opt->name,
>> - opt->type != VSH_OT_BOOL ? _("optdata") :
>> _("bool"),
>> - opt->type != VSH_OT_BOOL ? arg->data :
>> _("(none)"));
>> +    if (ctl)
> 
> Don't you mean (!partial) here?  This change looks weird otherwise.

Oh right.

> 
> After reading the following patches I see that you call this with ctl ==
> NULL
> (from readline).  That makes sense, but there are few more places where
> ctl is
> used even when partial != NULL and that needs to be handled correctly.

Like what? vshError() can handle if ctl is NULL, and subsequently
vshMalloc(), vshStrdup(), vshCmddefGetOption() and vshCommandCheckOpts()
can too.

Michal

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

Re: [libvirt] [PATCH] tests: Break symlink loop

[Martin Kletzander]

On Thu, Jan 11, 2018 at 08:41:47AM -0500, Andrea Bolognani wrote:

distcheck, and possibly more stuff, breaks because of it.



ACK under the build-breaker rule


Signed-off-by: Andrea Bolognani 
---
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0 | 1 -
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0 | 1 -
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0 | 1 -
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0 | 1 -
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0 | 1 -
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0 | 1 -
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0 | 1 -
tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0 | 1 -
8 files changed, 8 deletions(-)
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0
delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0

diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
--
2.14.3

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


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

Re: [libvirt] [PATCH 5/9] resctrl: Add functions to work with resctrl allocations

[Pavel Hrdina]

On Wed, Jan 03, 2018 at 04:05:38PM -0500, John Ferlan wrote:
> 
> 
> On 12/13/2017 10:39 AM, Martin Kletzander wrote:
> > With this commit we finally have a way to read and manipulate basic resctrl
> > settings.  Locking is done only on exposed functions that read/write from/to
> > resctrlfs.  Not in functions that are exposed in virresctrlpriv.h as those 
> > are
> > only supposed to be used from tests.
> > 
> > Signed-off-by: Martin Kletzander 
> > ---
> >  src/Makefile.am   |2 +-
> >  src/libvirt_private.syms  |   11 +
> >  src/util/virresctrl.c | 1041 
> > -
> >  src/util/virresctrl.h |   62 +++
> >  src/util/virresctrlpriv.h |   27 ++
> >  5 files changed, 1141 insertions(+), 2 deletions(-)
> >  create mode 100644 src/util/virresctrlpriv.h
> > 
> 
> Geez, as referenced by the cover letter, I'm glad this is the non way
> too complicated parts of the code (tongue firmly planted in my cheek
> with the obligatory eye roll emoji).
> 
> So, IIUC virResctrlInfoPtr is the "host" world and virResctrlAllocPtr is
> the "guest" world, right?  If so might be something to add to the commit
> messages to make things just slightly more clear.
> 
> Lots of code here - hopefully another set of eyes can look at this too -
> I'm sure I'll miss something as it's been very difficult to review this
> in one (long) session...
> 
> Note to self, no sense in grep-ing for "alloc" or "free" any more after
> this code is pushed as they're liberally used.  Kind of funny to see
> "alloc_free" in the same line ;-)
> 
> The other usage that was really confusing is @cache w/ @[n]masks and
> @[n]sizes.  It seems to be used as the array index for both and it's
> never really crystal clear over the relationship between masks and sizes.
> 
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 4c022d1e44b9..9b4fd0c1d597 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -167,7 +167,7 @@ UTIL_SOURCES = \
> > util/virprocess.c util/virprocess.h \
> > util/virqemu.c util/virqemu.h \
> > util/virrandom.h util/virrandom.c \
> > -   util/virresctrl.h util/virresctrl.c \
> > +   util/virresctrl.h util/virresctrl.c util/virresctrlpriv.h \
> > util/virrotatingfile.h util/virrotatingfile.c \
> > util/virscsi.c util/virscsi.h \
> > util/virscsihost.c util/virscsihost.h \
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 1a4f304f5e1f..a8009d913669 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -2548,6 +2548,17 @@ virRandomInt;
> >  # util/virresctrl.h
> >  virCacheTypeFromString;
> >  virCacheTypeToString;
> > +virResctrlAllocAddPID;
> > +virResctrlAllocCreate;
> > +virResctrlAllocForeachSize;
> > +virResctrlAllocFormat;
> > +virResctrlAllocGetFree;
> > +virResctrlAllocNew;
> > +virResctrlAllocRemove;
> > +virResctrlAllocSetID;
> > +virResctrlAllocSetSize;
> > +virResctrlAllocUpdateMask;
> > +virResctrlAllocUpdateSize;
> >  virResctrlGetInfo;
> >  virResctrlInfoGetCache;
> >  virResctrlInfoNew;
> > diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> > index a681322905be..32ab84b6f121 100644
> > --- a/src/util/virresctrl.c
> > +++ b/src/util/virresctrl.c
> > @@ -18,8 +18,12 @@
> >  
> >  #include 
> >  
> > -#include "virresctrl.h"
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> >  
> > +#include "virresctrlpriv.h"
> >  #include "c-ctype.h"
> >  #include "count-one-bits.h"
> >  #include "viralloc.h"
> > @@ -151,6 +155,156 @@ virResctrlInfoNew(void)
> >  }
> >  
> >  
> > +/* Alloc-related definitions and AllocClass-related functions */
> > +typedef struct _virResctrlAllocPerType virResctrlAllocPerType;
> > +typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr;
> > +struct _virResctrlAllocPerType {
> > +/* There could be bool saying whether this is set or not, but since 
> > everything
> > + * in virResctrlAlloc (and most of libvirt) goes with pointer arrays 
> > we would
> > + * have to have one more level of allocation anyway, so this stays 
> > faithful to
> > + * the concept */
> > +unsigned long long **sizes;
> > +size_t nsizes;
> 
> Perhaps something I should have noted in patch 2 - these @sizes are what
> exactly?  Is it a page size or just a "max size" in ?bytes for something
> stored in the cache?

It's an array that stores allocation size per CPU cache, since there
can be multiple CPUs in the host system.

> > +
> > +/* Mask for each cache */
> > +virBitmapPtr *masks;
> > +size_t nmasks;
> 
> And of course, what does the mask represent?

The mask represents how the cache allocation is written into the
schemata file and it is based on the cache allocation size.
Let's say that you have L3 cache with size 15 MiB where granularity
of cache allocation is 768 KiB but minimal allocation is 1536 KiB.
This means that you have 20bit 

[libvirt] [PATCH] tests: Break symlink loop

[Andrea Bolognani]

distcheck, and possibly more stuff, breaks because of it.

Signed-off-by: Andrea Bolognani 
---
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0 | 1 -
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0 | 1 -
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0 | 1 -
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0 | 1 -
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0 | 1 -
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0 | 1 -
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0 | 1 -
 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0 | 1 -
 8 files changed, 8 deletions(-)
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0
 delete mode 12 tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0

diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu0/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu1/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu2/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu3/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu4/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu5/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu6/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
diff --git a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0 
b/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0
deleted file mode 12
index 222b6af32..0
--- a/tests/virhostcpudata/linux-rhel74-moonshot/cpu/cpu7/node0
+++ /dev/null
@@ -1 +0,0 @@
-../../node/node0
\ No newline at end of file
-- 
2.14.3

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

Re: [libvirt] [PATCH v2] domain_conf: skip boot order check of CD-ROM or floppy device when change-media

[Ján Tomko]

On Thu, Jan 11, 2018 at 06:16:37PM +0800, Chen Hanxiao wrote:

From: Chen Hanxiao 

If we insert or eject a CD-ROM/floppy device by:
'virsh change-media VM --eject/--insert some.iso --live',
and the original CD-ROM device was configed with a boot order,
we may get:
 unsupported configuration: boot order 2 is already used by another device

We just updated 'source file' section rather than hotplug a new device.
This check should be skipped in this case.



Attempting to change the boot index on update won't work and should be
forbidden, as stated in the review for v1:
https://www.redhat.com/archives/libvir-list/2018-January/msg00178.html


Signed-off-by: Chen Hanxiao 
---
v2:
 commit message updated
 remove ATTRIBUTE_UNUSED from @device

src/conf/domain_conf.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1c25060f..e006cea0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26881,17 +26881,26 @@ virDomainDeviceIsUSB(virDomainDeviceDefPtr dev)

static int
virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED,
-  virDomainDeviceDefPtr device 
ATTRIBUTE_UNUSED,
+  virDomainDeviceDefPtr device,
  virDomainDeviceInfoPtr info,
  void *opaque)
{
virDomainDeviceInfoPtr newinfo = opaque;
+virDomainDiskDefPtr disk = device->data.disk;
+int disk_device = disk->device;

if (info->bootIndex == newinfo->bootIndex) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("boot order %u is already used by another device"),
-   newinfo->bootIndex);
-return -1;
+/* Skip check for insert or eject CD-ROM device */
+if (disk_device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
+disk_device == VIR_DOMAIN_DISK_DEVICE_CDROM) {


Even though cdrom hotplug is not supported by libvirt, assuming that
we're dealing with an update just because of the device type is wrong:
https://www.redhat.com/archives/libvir-list/2018-January/msg00180.html

virDomainDefCompatibleDevice should be aware of the operation (attach
vs. update) and behave accordingly (forbid duplicit bootindexes for
attach and a bootindex change for update)

Jan

+VIR_DEBUG("Skip boot index check for floppy or CDROM");
+return 0;
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("boot order %u is already used by another 
device"),
+   newinfo->bootIndex);
+return -1;
+}
}
return 0;
}
--
2.14.3

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


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

Re: [libvirt] [PATCH 08/18] vshCommandOpt: Allow caller avoiding assert()

[Martin Kletzander]

On Thu, Jan 11, 2018 at 08:09:25AM -0500, John Ferlan wrote:



On 01/11/2018 07:55 AM, Martin Kletzander wrote:

On Thu, Jan 11, 2018 at 06:02:58AM -0500, John Ferlan wrote:



On 01/11/2018 05:50 AM, Martin Kletzander wrote:

On Tue, Jan 02, 2018 at 06:12:01PM +0100, Michal Privoznik wrote:

In the future, completer callbacks will receive partially parsed
command (and thus possibly incomplete). However, we still want
them to use command options fetching APIs we already have (e.g.
vshCommandOpt*()) and at the same time don't report any errors
(nor call any asserts).

Signed-off-by: Michal Privoznik 
---
tools/vsh.c | 7 ---
tools/vsh.h | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index ebc8d9cb1..d27acb95b 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
 * to the option if found, 0 with *OPT set to NULL if the name is
 * valid and the option is not required, -1 with *OPT set to NULL if
 * the option is required but not present, and assert if NAME is not
- * valid (which indicates a programming error).  No error messages are
- * issued if a value is returned.
+ * valid (which indicates a programming error) unless cmd->skipChecks
+ * is set. No error messages are issued if a value is returned.
 */
static int
vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
@@ -829,7 +829,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name,
vshCmdOpt **opt,
    /* See if option is valid and/or required.  */
    *opt = NULL;
    while (valid) {
-    assert(valid->name);
+    if (!cmd->skipChecks)
+    assert(valid->name);


This can segfault when cmd->skipChecks == False && valid->name == NULL,
which is what the assert() guarded before.

So either STREQ_NULLABLE or another if.



Hmmm... Also see:

https://www.redhat.com/archives/libvir-list/2017-December/msg00605.html

it's related somewhat...



I don't see how, this is all wrapped in `while (valid)`


The other patch is "after" the loop.

Look at the entire context... although we know it's a software
engineering error to not have some sort of match, some compiler believes
we can exit the "while (valid)" loop with "valid == NULL", follwed by
the next assert which dereferences @valid without asserting if valid is
non-NULL.



Oh, I guess that patch from Marc should be pushed (I still didn't get to
s many patches on the list) and this should then handle addition to
that change as well.  I got confused by your vague "somewhat related" =)


I didn't say it was exactly related, just "related somewhat".

John

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


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

Re: [libvirt] [PATCH 08/18] vshCommandOpt: Allow caller avoiding assert()

[John Ferlan]

On 01/11/2018 07:55 AM, Martin Kletzander wrote:
> On Thu, Jan 11, 2018 at 06:02:58AM -0500, John Ferlan wrote:
>>
>>
>> On 01/11/2018 05:50 AM, Martin Kletzander wrote:
>>> On Tue, Jan 02, 2018 at 06:12:01PM +0100, Michal Privoznik wrote:
 In the future, completer callbacks will receive partially parsed
 command (and thus possibly incomplete). However, we still want
 them to use command options fetching APIs we already have (e.g.
 vshCommandOpt*()) and at the same time don't report any errors
 (nor call any asserts).

 Signed-off-by: Michal Privoznik 
 ---
 tools/vsh.c | 7 ---
 tools/vsh.h | 3 ++-
 2 files changed, 6 insertions(+), 4 deletions(-)

 diff --git a/tools/vsh.c b/tools/vsh.c
 index ebc8d9cb1..d27acb95b 100644
 --- a/tools/vsh.c
 +++ b/tools/vsh.c
 @@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
  * to the option if found, 0 with *OPT set to NULL if the name is
  * valid and the option is not required, -1 with *OPT set to NULL if
  * the option is required but not present, and assert if NAME is not
 - * valid (which indicates a programming error).  No error messages are
 - * issued if a value is returned.
 + * valid (which indicates a programming error) unless cmd->skipChecks
 + * is set. No error messages are issued if a value is returned.
  */
 static int
 vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
 @@ -829,7 +829,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name,
 vshCmdOpt **opt,
     /* See if option is valid and/or required.  */
     *opt = NULL;
     while (valid) {
 -    assert(valid->name);
 +    if (!cmd->skipChecks)
 +    assert(valid->name);
>>>
>>> This can segfault when cmd->skipChecks == False && valid->name == NULL,
>>> which is what the assert() guarded before.
>>>
>>> So either STREQ_NULLABLE or another if.
>>>
>>
>> Hmmm... Also see:
>>
>> https://www.redhat.com/archives/libvir-list/2017-December/msg00605.html
>>
>> it's related somewhat...
>>
> 
> I don't see how, this is all wrapped in `while (valid)`

The other patch is "after" the loop.

Look at the entire context... although we know it's a software
engineering error to not have some sort of match, some compiler believes
we can exit the "while (valid)" loop with "valid == NULL", follwed by
the next assert which dereferences @valid without asserting if valid is
non-NULL.

I didn't say it was exactly related, just "related somewhat".

John

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

Re: [libvirt] [PATCH 08/18] vshCommandOpt: Allow caller avoiding assert()

[Martin Kletzander]

On Thu, Jan 11, 2018 at 06:02:58AM -0500, John Ferlan wrote:



On 01/11/2018 05:50 AM, Martin Kletzander wrote:

On Tue, Jan 02, 2018 at 06:12:01PM +0100, Michal Privoznik wrote:

In the future, completer callbacks will receive partially parsed
command (and thus possibly incomplete). However, we still want
them to use command options fetching APIs we already have (e.g.
vshCommandOpt*()) and at the same time don't report any errors
(nor call any asserts).

Signed-off-by: Michal Privoznik 
---
tools/vsh.c | 7 ---
tools/vsh.h | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index ebc8d9cb1..d27acb95b 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
 * to the option if found, 0 with *OPT set to NULL if the name is
 * valid and the option is not required, -1 with *OPT set to NULL if
 * the option is required but not present, and assert if NAME is not
- * valid (which indicates a programming error).  No error messages are
- * issued if a value is returned.
+ * valid (which indicates a programming error) unless cmd->skipChecks
+ * is set. No error messages are issued if a value is returned.
 */
static int
vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
@@ -829,7 +829,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name,
vshCmdOpt **opt,
    /* See if option is valid and/or required.  */
    *opt = NULL;
    while (valid) {
-    assert(valid->name);
+    if (!cmd->skipChecks)
+    assert(valid->name);


This can segfault when cmd->skipChecks == False && valid->name == NULL,
which is what the assert() guarded before.

So either STREQ_NULLABLE or another if.



Hmmm... Also see:

https://www.redhat.com/archives/libvir-list/2017-December/msg00605.html

it's related somewhat...



I don't see how, this is all wrapped in `while (valid)`


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

Re: [libvirt] [PATCH] qemuDomainRemoveMemoryDevice: unlink() memory backing file

[Michal Privoznik]

On 01/11/2018 01:36 PM, Peter Krempa wrote:
> On Thu, Jan 11, 2018 at 13:24:57 +0100, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1461214
>>
>> Since fec8f9c49af we try to use predictable file names for
>> 'memory-backend-file' objects. But that made us provide full path
>> to qemu when hot plugging the object while previously we provided
>> merely a directory. But this makes qemu behave differently. If
>> qemu sees a path terminated with a directory it calls mkstemp()
>> and unlinks the file immediately. But if it sees full path it
>> just calls open(path, O_CREAT ..); and never unlinks the file.
>> Therefore it's up to libvirt to unlink the file and not leave it
>> behind.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>
>> Zack, can you please check if this patch is suitable for your use cases?
>>
>>  src/qemu/qemu_hotplug.c |  3 +++
>>  src/qemu/qemu_process.c | 26 ++
>>  src/qemu/qemu_process.h |  4 
>>  3 files changed, 33 insertions(+)
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 6dc16a105..f26e2ca60 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -3894,6 +3894,9 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
>>  if (qemuDomainNamespaceTeardownMemory(vm, mem) <  0)
>>  VIR_WARN("Unable to remove memory device from /dev");
>>  
>> +if (qemuProcessDestroyMemoryBackingPath(driver, vm, mem) < 0)
>> +VIR_WARN("Unable to destroy memory backing path");
>> +
>>  virDomainMemoryDefFree(mem);
> 
> This will not call the function when we shut down qemu. Is the full
> directory removed in that case?
> 

Yes, from qemuProcessStop() we call qemuProcessBuildDestroyMemoryPaths()
which in turn calls virFileDeleteTree() over all domain specific
hugepages paths and memoryBacking ones.

Michal

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

Re: [libvirt] [PATCH] qemuDomainRemoveMemoryDevice: unlink() memory backing file

[Michal Privoznik]

On 01/11/2018 01:32 PM, Daniel P. Berrange wrote:
> On Thu, Jan 11, 2018 at 01:24:57PM +0100, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1461214
>>
>> Since fec8f9c49af we try to use predictable file names for
>> 'memory-backend-file' objects. But that made us provide full path
>> to qemu when hot plugging the object while previously we provided
>> merely a directory. But this makes qemu behave differently. If
>> qemu sees a path terminated with a directory it calls mkstemp()
>> and unlinks the file immediately. But if it sees full path it
>> just calls open(path, O_CREAT ..); and never unlinks the file.
>> Therefore it's up to libvirt to unlink the file and not leave it
>> behind.
> 
> IIUC, this unlinks the file when QEMU shuts down (or the mem device is
> unplugged).
> 
> Is there any benefit and/or downside to doing the same as QEMU and
> unlinking it immediately after QEMU has opened it ? I guess figuring
> out the right time might be hard todo race-free.

Yeah, I thought about that race too. Also, since we use
memory-backing-file whenever memoryBacking/access/@mode='shared' or
memoryBacking/source/@type='file' is used I figured that whoever set
those might want to do something with the files (e.g. read them?). But
if we unlink them right away we would be only making it harder for users
to get to the files. But I don't have strong preference either way.

Michal

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

Re: [libvirt] [PATCH] qemuDomainRemoveMemoryDevice: unlink() memory backing file

[Peter Krempa]

On Thu, Jan 11, 2018 at 13:24:57 +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1461214
> 
> Since fec8f9c49af we try to use predictable file names for
> 'memory-backend-file' objects. But that made us provide full path
> to qemu when hot plugging the object while previously we provided
> merely a directory. But this makes qemu behave differently. If
> qemu sees a path terminated with a directory it calls mkstemp()
> and unlinks the file immediately. But if it sees full path it
> just calls open(path, O_CREAT ..); and never unlinks the file.
> Therefore it's up to libvirt to unlink the file and not leave it
> behind.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> Zack, can you please check if this patch is suitable for your use cases?
> 
>  src/qemu/qemu_hotplug.c |  3 +++
>  src/qemu/qemu_process.c | 26 ++
>  src/qemu/qemu_process.h |  4 
>  3 files changed, 33 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6dc16a105..f26e2ca60 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3894,6 +3894,9 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
>  if (qemuDomainNamespaceTeardownMemory(vm, mem) <  0)
>  VIR_WARN("Unable to remove memory device from /dev");
>  
> +if (qemuProcessDestroyMemoryBackingPath(driver, vm, mem) < 0)
> +VIR_WARN("Unable to destroy memory backing path");
> +
>  virDomainMemoryDefFree(mem);

This will not call the function when we shut down qemu. Is the full
directory removed in that case?


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

Re: [libvirt] [PATCH] qemuDomainRemoveMemoryDevice: unlink() memory backing file

[Daniel P. Berrange]

On Thu, Jan 11, 2018 at 01:24:57PM +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1461214
> 
> Since fec8f9c49af we try to use predictable file names for
> 'memory-backend-file' objects. But that made us provide full path
> to qemu when hot plugging the object while previously we provided
> merely a directory. But this makes qemu behave differently. If
> qemu sees a path terminated with a directory it calls mkstemp()
> and unlinks the file immediately. But if it sees full path it
> just calls open(path, O_CREAT ..); and never unlinks the file.
> Therefore it's up to libvirt to unlink the file and not leave it
> behind.

IIUC, this unlinks the file when QEMU shuts down (or the mem device is
unplugged).

Is there any benefit and/or downside to doing the same as QEMU and
unlinking it immediately after QEMU has opened it ? I guess figuring
out the right time might be hard todo race-free.

> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> Zack, can you please check if this patch is suitable for your use cases?
> 
>  src/qemu/qemu_hotplug.c |  3 +++
>  src/qemu/qemu_process.c | 26 ++
>  src/qemu/qemu_process.h |  4 
>  3 files changed, 33 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6dc16a105..f26e2ca60 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3894,6 +3894,9 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
>  if (qemuDomainNamespaceTeardownMemory(vm, mem) <  0)
>  VIR_WARN("Unable to remove memory device from /dev");
>  
> +if (qemuProcessDestroyMemoryBackingPath(driver, vm, mem) < 0)
> +VIR_WARN("Unable to destroy memory backing path");
> +
>  virDomainMemoryDefFree(mem);
>  
>  /* fix the balloon size */
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 1a0923af3..73624eefe 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3467,6 +3467,32 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr 
> driver,
>  }
>  
>  
> +int
> +qemuProcessDestroyMemoryBackingPath(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +virDomainMemoryDefPtr mem)
> +{
> +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +char *path = NULL;
> +int ret = -1;
> +
> +if (qemuGetMemoryBackingPath(vm->def, cfg, mem->info.alias, ) < 0)
> +goto cleanup;
> +
> +if (unlink(path) < 0 &&
> +errno != ENOENT) {
> +virReportSystemError(errno, _("Unable to remove %s"), path);
> +goto cleanup;
> +}
> +
> +ret = 0;
> + cleanup:
> +VIR_FREE(path);
> +virObjectUnref(cfg);
> +return ret;
> +}
> +
> +
>  static int
>  qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
>  virDomainGraphicsDefPtr graphics,
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index cd9a72031..3fc7d6c85 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -43,6 +43,10 @@ int qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr 
> driver,
> virDomainMemoryDefPtr mem,
> bool build);
>  
> +int qemuProcessDestroyMemoryBackingPath(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +virDomainMemoryDefPtr mem);
> +
>  void qemuProcessAutostartAll(virQEMUDriverPtr driver);
>  void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver);
>  
> -- 
> 2.13.6
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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 5/5] qemu: implement the 'libvirt_qemu' shim for launching guests externally

[Daniel P. Berrange]

On Tue, Jan 09, 2018 at 03:13:41PM +0100, Martin Kletzander wrote:
> On Wed, Dec 20, 2017 at 04:47:50PM +, Daniel P. Berrange wrote:
> > This introduces a new binary 'libvirt_qemu' which can be used to launch
> > guests externally from libvirtd. eg
> > 
> >  libvirt_qemu -c qemu:///system /path/to/xml/file
> > 
> > This will launch a guest from the requested XML file and then connect to
> > qemu:///system to register it with libvirtd. At this point all normal
> > libvirtd operations should generally work.
> > 
> > NB, this impl has many flaws:
> > 
> > - We don't generate a unique 'id', so all guests will get id==1.
> >   Surprisingly this doesn't break the world.
> > 
> > - Tracking of unique name + UUIDs is *not* race free.
> > 
> > - No tracking/allocation of shared resource state (PCI devices,
> >   etc)
> > 
> 
> You will also not get the same events as you would when starting the domain
> as usual.

Hmm, yes, interesting. If someone is connected to libvirtd watching events
they'll  not get any startup events. That's something we'd need to fix.

> > Most other functionality works, but might have expected results.
> > 
> 
> If I start a domain with a network, it segfaults.  I'll see later if that's my
> fault or not.  It probably (and hopefully) is.

Not intentional if that's happening !


> > - The libvirt_qemu will exit if startup fails, however, it
> >   won't exit when QEMU later shuts down - you must listen
> >   for libvirtd events to detect that right now. We'll
> >   fix that
> > 
> > - Killing the libvirt_qemu shim will not kill the QEMU
> >   process. You must kill via the libvirt API as normal.
> >   This won't change, because we need to be able to
> >   ultimately restart the libvirt_qemu shim in order to
> >   apply software updates to running instance.
> >   We might wire up a special signal though to let
> >   you kill libvirt_qemu & take out QEMU at same time
> >   eg SIGQUIT or something like that perhaps.
> > 
> 
> IMHO we should use standard signals to do standard things.  TERM should
> gracefully shutdown the domain.  Not in PoC, but later.  That way users
> can treat the shim process as other processes.

I guess we can use SIGUSR1 to trigger restart of the shim while
leaving QEMU running.


> > +if WITH_QEMU
> > +if WITH_LIBVIRTD
> > +libexec_PROGRAMS += libvirt_qemu
> > +
> 
> Shouldn't I be able to launch this as a user without mgmt app?  It
> should go to bin_PROGRAMS, I presume.

Yes

> > +libvirt_qemu_SOURCES = \
> > +   $(QEMU_CONTROLLER_SOURCES) \
> > +   $(DATATYPES_SOURCES)
> > +libvirt_qemu_LDFLAGS = \
> > +   $(AM_LDFLAGS) \
> > +   $(PIE_LDFLAGS) \
> > +   $(NULL)
> > +libvirt_qemu_LDADD = \
> > +   libvirt.la \
> > +   libvirt-qemu.la \
> > +   libvirt_driver_qemu_impl.la
> > +if WITH_NETWORK
> > +libvirt_qemu_LDADD += libvirt_driver_network_impl.la
> > +endif WITH_NETWORK
> > +if WITH_STORAGE
> > +libvirt_qemu_LDADD += libvirt_driver_storage_impl.la
> > +endif WITH_STORAGE
> 
> > +libvirt_qemu_LDADD += ../gnulib/lib/libgnu.la $(LIBSOCKET)
> 
> This is not part of any condition, should be up there with libvirt.la
> just for clarity.

IIRC, libgnu.la needs to be the last library listed

> > +static void
> > +virQEMUControllerDriverFree(virQEMUDriverPtr driver)
> > +{
> > +if (!driver)
> > +return;
> > +
> > +virObjectUnref(driver->config);
> > +virObjectUnref(driver->hostdevMgr);
> > +virHashFree(driver->sharedDevices);
> > +virObjectUnref(driver->caps);
> > +virObjectUnref(driver->qemuCapsCache);
> > +
> > +virObjectUnref(driver->domains);
> > +virObjectUnref(driver->remotePorts);
> > +virObjectUnref(driver->webSocketPorts);
> > +virObjectUnref(driver->migrationPorts);
> > +virObjectUnref(driver->migrationErrors);
> > +
> > +virObjectUnref(driver->xmlopt);
> > +
> > +virSysinfoDefFree(driver->hostsysinfo);
> > +
> > +virObjectUnref(driver->closeCallbacks);
> > +
> > +VIR_FREE(driver->qemuImgBinary);
> > +
> > +virObjectUnref(driver->securityManager);
> > +
> > +ebtablesContextFree(driver->ebtables);
> > +
> > +virLockManagerPluginUnref(driver->lockManager);
> > +
> > +virMutexDestroy(>lock);
> > +VIR_FREE(driver);
> > +}
> > +
> > +static virQEMUDriverPtr virQEMUControllerNewDriver(bool privileged)
> 
> This is very similar to what I did, but I just exported this function
> privately si that there is less duplication of code and I added an enum
> that is used as a parameter instead of the bool @privileged.  It is a
> tristate (user, system, shim) that special-cases the shim slightly.

Yeah, there's definitely much more duplication here that I would like.
I would expect to refactor this better.

> The reason behind that is that I kind of presumed we need to be able to
> launch system QEMUs with non-root user.  Thinking about it I'm not sure
> that's the case for kubevirt, but it definitely is for others.  The way
> 

[libvirt] [PATCH] qemuDomainRemoveMemoryDevice: unlink() memory backing file

[Michal Privoznik]

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

Since fec8f9c49af we try to use predictable file names for
'memory-backend-file' objects. But that made us provide full path
to qemu when hot plugging the object while previously we provided
merely a directory. But this makes qemu behave differently. If
qemu sees a path terminated with a directory it calls mkstemp()
and unlinks the file immediately. But if it sees full path it
just calls open(path, O_CREAT ..); and never unlinks the file.
Therefore it's up to libvirt to unlink the file and not leave it
behind.

Signed-off-by: Michal Privoznik 
---

Zack, can you please check if this patch is suitable for your use cases?

 src/qemu/qemu_hotplug.c |  3 +++
 src/qemu/qemu_process.c | 26 ++
 src/qemu/qemu_process.h |  4 
 3 files changed, 33 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6dc16a105..f26e2ca60 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3894,6 +3894,9 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
 if (qemuDomainNamespaceTeardownMemory(vm, mem) <  0)
 VIR_WARN("Unable to remove memory device from /dev");
 
+if (qemuProcessDestroyMemoryBackingPath(driver, vm, mem) < 0)
+VIR_WARN("Unable to destroy memory backing path");
+
 virDomainMemoryDefFree(mem);
 
 /* fix the balloon size */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1a0923af3..73624eefe 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3467,6 +3467,32 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr 
driver,
 }
 
 
+int
+qemuProcessDestroyMemoryBackingPath(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virDomainMemoryDefPtr mem)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+char *path = NULL;
+int ret = -1;
+
+if (qemuGetMemoryBackingPath(vm->def, cfg, mem->info.alias, ) < 0)
+goto cleanup;
+
+if (unlink(path) < 0 &&
+errno != ENOENT) {
+virReportSystemError(errno, _("Unable to remove %s"), path);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(path);
+virObjectUnref(cfg);
+return ret;
+}
+
+
 static int
 qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
 virDomainGraphicsDefPtr graphics,
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index cd9a72031..3fc7d6c85 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -43,6 +43,10 @@ int qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr 
driver,
virDomainMemoryDefPtr mem,
bool build);
 
+int qemuProcessDestroyMemoryBackingPath(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virDomainMemoryDefPtr mem);
+
 void qemuProcessAutostartAll(virQEMUDriverPtr driver);
 void qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver);
 
-- 
2.13.6

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

Re: [libvirt] [PATCH 1/5] conf: allow different resource registration modes

[Daniel P. Berrange]

On Thu, Jan 11, 2018 at 12:14:21PM +, Daniel P. Berrange wrote:
> On Tue, Jan 09, 2018 at 01:43:39PM +0100, Martin Kletzander wrote:
> > I'm so sorry for not getting to this earlier.  I though I'll get to this 
> > over
> > the holidays, but they were very busy with no free time for me.
> > 
> > On Wed, Dec 20, 2017 at 04:47:46PM +, Daniel P. Berrange wrote:
> > > Currently the QEMU driver has three ways of setting up cgroups. It either
> > > skips them entirely (if non-root), or uses systemd-machined, or uses
> > > cgroups directly.
> > > 
> > 
> > So what we are trying to fix here is that all of the variations don't 
> > create the
> > same structure.  So it needs to be clear for the mgmt app to guess^Wknow
> > correctly where the domain is in the cgroup hierarchy.
> > 
> > > It is further possible to register directly with systemd and bypass
> > > machined. We don't support this by systemd-nsspawn does and we ought
> > > to.
> > 
> > But what's the benefit of that?
> 
> Systemd recommends that you only register with machined, if you are running a
> full OS install in the machine.  IOW, if you are using LXC / QEMU for 
> sandboxing
> individual applications, instead of full OS, then you should not register with
> machined. So this is something libvirt sandbox ought to be able to request, 
> for
> exmaple.
> 
> > 
> > > This change adds ability to configure the mechanism for registering
> > > resources between all these options explicitly. via
> > > 
> > >  
> > > 
> > 
> > I understand what you are trying to fix, but I don't quite follow why we 
> > should
> > expose that.  Can't we guess some of them easily?  Or are you making this 
> > part
> > of the PoC, but then removing it later?
> 
> No, I intend this bit to be fully supported long term.
> 
>  * none - use this to just inherit current placement of the caller.
>   This is akin to turning off all use of cgroups in qemu.conf
> if launching from libvirtd, causing currently placement of
> libvirtd in cgroups to be inherited by VMs.
> 
>   This is what we'll also need with the shim for KubeVirt's
> needs
> 
>  * machined - register with machined - this is what we do today, if we detect
>   that machined is available
> 
>  * direct - create cgroups directly - this is what we do if machined is not
> installed, or we have no dbus connection available.
> 
>  * systemd - register with systemd only, not with machined - see above
>  rationale
> 
> It is unlikely that apps would use 'direct' if running on a systemd based
> host. Likewise using machined/systemd on a non-systemd host is not possible.
> 
> But that still leaves a choice of 2/3 options that are viable and cannot be
> automatically determined, and are reasonable to vary per-VM.

Oh, I also meant to say that I would expect us to update this in the live
XML, if the user/app left it unspecified. This would allow the app to
determine whether libvirt has actually activated cgroup support or not,
and thus whether resource mgmt apis/config will work

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

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

Re: [libvirt] [PATCH 1/5] conf: allow different resource registration modes

[Daniel P. Berrange]

On Tue, Jan 09, 2018 at 01:43:39PM +0100, Martin Kletzander wrote:
> I'm so sorry for not getting to this earlier.  I though I'll get to this over
> the holidays, but they were very busy with no free time for me.
> 
> On Wed, Dec 20, 2017 at 04:47:46PM +, Daniel P. Berrange wrote:
> > Currently the QEMU driver has three ways of setting up cgroups. It either
> > skips them entirely (if non-root), or uses systemd-machined, or uses
> > cgroups directly.
> > 
> 
> So what we are trying to fix here is that all of the variations don't create 
> the
> same structure.  So it needs to be clear for the mgmt app to guess^Wknow
> correctly where the domain is in the cgroup hierarchy.
> 
> > It is further possible to register directly with systemd and bypass
> > machined. We don't support this by systemd-nsspawn does and we ought
> > to.
> 
> But what's the benefit of that?

Systemd recommends that you only register with machined, if you are running a
full OS install in the machine.  IOW, if you are using LXC / QEMU for sandboxing
individual applications, instead of full OS, then you should not register with
machined. So this is something libvirt sandbox ought to be able to request, for
exmaple.

> 
> > This change adds ability to configure the mechanism for registering
> > resources between all these options explicitly. via
> > 
> >  
> > 
> 
> I understand what you are trying to fix, but I don't quite follow why we 
> should
> expose that.  Can't we guess some of them easily?  Or are you making this part
> of the PoC, but then removing it later?

No, I intend this bit to be fully supported long term.

 * none - use this to just inherit current placement of the caller.
  This is akin to turning off all use of cgroups in qemu.conf
  if launching from libvirtd, causing currently placement of
  libvirtd in cgroups to be inherited by VMs.

  This is what we'll also need with the shim for KubeVirt's
  needs

 * machined - register with machined - this is what we do today, if we detect
  that machined is available

 * direct - create cgroups directly - this is what we do if machined is not
installed, or we have no dbus connection available.

 * systemd - register with systemd only, not with machined - see above
 rationale

It is unlikely that apps would use 'direct' if running on a systemd based
host. Likewise using machined/systemd on a non-systemd host is not possible.

But that still leaves a choice of 2/3 options that are viable and cannot be
automatically determined, and are reasonable to vary per-VM.

> For now I am OK with that, but I would rather see that as a part of qemu.conf
> (or libvirtd.conf), not really in the XML.  OK for the PoC, though.



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 08/18] vshCommandOpt: Allow caller avoiding assert()

[John Ferlan]

On 01/11/2018 05:50 AM, Martin Kletzander wrote:
> On Tue, Jan 02, 2018 at 06:12:01PM +0100, Michal Privoznik wrote:
>> In the future, completer callbacks will receive partially parsed
>> command (and thus possibly incomplete). However, we still want
>> them to use command options fetching APIs we already have (e.g.
>> vshCommandOpt*()) and at the same time don't report any errors
>> (nor call any asserts).
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>> tools/vsh.c | 7 ---
>> tools/vsh.h | 3 ++-
>> 2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index ebc8d9cb1..d27acb95b 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
>>  * to the option if found, 0 with *OPT set to NULL if the name is
>>  * valid and the option is not required, -1 with *OPT set to NULL if
>>  * the option is required but not present, and assert if NAME is not
>> - * valid (which indicates a programming error).  No error messages are
>> - * issued if a value is returned.
>> + * valid (which indicates a programming error) unless cmd->skipChecks
>> + * is set. No error messages are issued if a value is returned.
>>  */
>> static int
>> vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>> @@ -829,7 +829,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name,
>> vshCmdOpt **opt,
>>     /* See if option is valid and/or required.  */
>>     *opt = NULL;
>>     while (valid) {
>> -    assert(valid->name);
>> +    if (!cmd->skipChecks)
>> +    assert(valid->name);
> 
> This can segfault when cmd->skipChecks == False && valid->name == NULL,
> which is what the assert() guarded before.
> 
> So either STREQ_NULLABLE or another if.
> 

Hmmm... Also see:

https://www.redhat.com/archives/libvir-list/2017-December/msg00605.html

it's related somewhat...

John

>>     if (STREQ(name, valid->name))
>>     break;
>>     valid++;
>> diff --git a/tools/vsh.h b/tools/vsh.h
>> index 8f7df9ff8..112b1b57d 100644
>> --- a/tools/vsh.h
>> +++ b/tools/vsh.h
>> @@ -188,7 +188,8 @@ struct _vshCmdDef {
>> struct _vshCmd {
>>     const vshCmdDef *def;   /* command definition */
>>     vshCmdOpt *opts;    /* list of command arguments */
>> -    vshCmd *next;  /* next command */
>> +    vshCmd *next;   /* next command */
>> +    bool skipChecks;    /* skip validity checks when
>> retrieving opts */
>> };
>>
>> /*
>> -- 
>> 2.13.6
>>
>> -- 
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
> 
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

Re: [libvirt] [PATCH 00/18] Make bash completion great again

[Martin Kletzander]

On Tue, Jan 02, 2018 at 06:11:53PM +0100, Michal Privoznik wrote:

Technically, this is a v2 of [1], but this implements the feature from
different angle and therefore it's a start of new series.


What's implemented?
===
Auto completion for both interactive and non-interactive
virsh/virt-admin.


Known limitations
=
Currently, just options completers work. I mean, to bring up list
of domains you have to:

 virsh # start --domain 

Doing bare:

 virsh # start 

brings up list of --options. With the new approach implemented
here it should be easy to implement this. But that can be saved
for later.


How to test this?
=

Interactive completion should work out of the box. Just make sure
you're connected. Completers don't connect! You certainly don't
want ssh's 'Password:' prompt show on , do you?
Non-interactive completers do connect, but in order to avoid any
password prompts, /dev/stdin is closed before anything else is
done. In order to test it, just:

 libvirt.git $ source tools/bash-completion/vsh

Now, bash completion should work:

 libvirt.git $ ./tools/virsh -c qemu:///system start --domain 


Notes
=

As usual, you can find all the patches on my github [2]. I've
tried to work in all the review suggestions from v1. Due to
changes in design (reusing parse code instead of duplicating it)
not all suggestions were possible to work in though.

Michal



ACK to all if you fix the two possible NULL problems (04/18 and 08/18).


1: https://www.redhat.com/archives/libvir-list/2017-November/msg00213.html
2: https://github.com/zippy2/libvirt/tree/bash_autocomplete_v3  (yeah v3, don't 
ask)

Michal Privoznik (18):
 vsh: Drop useless check for opts != NULL
 vsh: Drop useless check for cmd != NULL
 vshCommandParse: Don't leak @tkdata
 vshCommandStringParse: Allow retrieving partial result
 vshReadlineParse: Drop code duplication
 vshReadlineParse: Escape returned results if needed
 vshReadlineParse: Use string list
 vshCommandOpt: Allow caller avoiding assert()
 util: Introduce virStringListMerge
 vsh: Fix vshCompleter signature
 vsh: Call vshCmdOptDef completer
 vsh: Prune string list returned by completer
 vsh: Filter --options
 vsh: Introduce complete command
 tools: Provide bash autompletion file
 virsh: Introduce virshDomainNameCompleter
 virsh: Introduce virshDomainInterfaceCompleter
 virt-admin: Introduce vshAdmServerCompleter

configure.ac |   3 +
libvirt.spec.in  |   3 +
m4/virt-bash-completion.m4   |  74 +
src/libvirt_private.syms |   1 +
src/util/virstring.c |  36 +++
src/util/virstring.h |   3 +
tools/Makefile.am|  40 ++-
tools/bash-completion/vsh|  67 +
tools/virsh-completer.c  | 150 ++
tools/virsh-completer.h  |  41 +++
tools/virsh-domain-monitor.c |  33 ++-
tools/virsh-domain.c | 186 ++--
tools/virsh-snapshot.c   |  24 +-
tools/virsh.c|   5 +-
tools/virsh.h|   7 +-
tools/virt-admin-completer.c |  76 +
tools/virt-admin-completer.h |  33 +++
tools/virt-admin.c   |  13 +-
tools/vsh.c  | 663 ++-
tools/vsh.h  |  23 +-
20 files changed, 1097 insertions(+), 384 deletions(-)
create mode 100644 m4/virt-bash-completion.m4
create mode 100644 tools/bash-completion/vsh
create mode 100644 tools/virsh-completer.c
create mode 100644 tools/virsh-completer.h
create mode 100644 tools/virt-admin-completer.c
create mode 100644 tools/virt-admin-completer.h

--
2.13.6

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


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

Re: [libvirt] [PATCH 16/18] virsh: Introduce virshDomainNameCompleter

[Martin Kletzander]

On Tue, Jan 02, 2018 at 06:12:09PM +0100, Michal Privoznik wrote:

Now that we have everything prepared let the fun begin. This
completer is very simple and returns domain names. Moreover,
depending on the command it can return just a subset of domains
(e.g. only running/paused/transient/.. ones).

Signed-off-by: Michal Privoznik 
---
tools/Makefile.am|   9 +++
tools/virsh-completer.c  |  90 +
tools/virsh-completer.h  |  33 
tools/virsh-domain-monitor.c |  30 +++
tools/virsh-domain.c | 182 ++-
tools/virsh-snapshot.c   |  24 +++---
tools/virsh.h|   7 +-
7 files changed, 256 insertions(+), 119 deletions(-)
create mode 100644 tools/virsh-completer.c
create mode 100644 tools/virsh-completer.h

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
new file mode 100644
index 0..4e32b882b
--- /dev/null
+++ b/tools/virsh-completer.c
@@ -0,0 +1,90 @@
+/*
+ * virsh-completer.c: virsh completer callbacks
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ *  Michal Privoznik 


These two spaces before the name with no indication what the name means (like
"Authors" that we have in other files) makes me wonder why do we have names here
and there. Publicity?


+ *
+ */
+
+#include 
+
+#include "virsh-completer.h"
+#include "virsh.h"
+#include "virsh-util.h"
+#include "internal.h"
+#include "viralloc.h"
+#include "virstring.h"
+
+
+char **
+virshDomainNameCompleter(vshControl *ctl,
+ const vshCmd *cmd ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+virshControlPtr priv = ctl->privData;
+virDomainPtr *domains = NULL;
+int ndomains = 0;
+size_t i = 0;
+char **ret = NULL;
+
+virCheckFlags(VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+  VIR_CONNECT_LIST_DOMAINS_INACTIVE |
+  VIR_CONNECT_LIST_DOMAINS_PERSISTENT |
+  VIR_CONNECT_LIST_DOMAINS_TRANSIENT |
+  VIR_CONNECT_LIST_DOMAINS_RUNNING |
+  VIR_CONNECT_LIST_DOMAINS_PAUSED |
+  VIR_CONNECT_LIST_DOMAINS_SHUTOFF |
+  VIR_CONNECT_LIST_DOMAINS_OTHER |
+  VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE |
+  VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE |
+  VIR_CONNECT_LIST_DOMAINS_AUTOSTART |
+  VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART |
+  VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT |
+  VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT,
+  NULL);
+
+if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+return NULL;
+
+if ((ndomains = virConnectListAllDomains(priv->conn, , flags)) < 0)
+return NULL;
+
+if (VIR_ALLOC_N(ret, ndomains + 1) < 0)
+goto error;
+
+for (i = 0; i < ndomains; i++) {
+const char *name = virDomainGetName(domains[i]);
+
+if (VIR_STRDUP(ret[i], name) < 0)
+goto error;
+
+virshDomainFree(domains[i]);
+}
+VIR_FREE(domains);
+
+return ret;
+ error:
+


Weird whitespace, I guess the label should've been one line lower.

I know, just bikeshedding.  Nothing against this patch.


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

Re: [libvirt] [PATCH 17/18] virsh: Introduce virshDomainInterfaceCompleter

[Martin Kletzander]

On Tue, Jan 02, 2018 at 06:12:10PM +0100, Michal Privoznik wrote:

For given domain fetch list of defined interfaces. This can be
used for commands like domif-getlink and others. If available,
the interface name is returned (e.g. "vnet0", usually available
only for running domains), if not the MAC address is returned.
Moreover, the detach-interface command requires only MAC address
and therefore we have new flag that forces the completer to
return just the MAC address.

Signed-off-by: Michal Privoznik 
---
tools/virsh-completer.c  | 60 
tools/virsh-completer.h  |  8 ++
tools/virsh-domain-monitor.c |  3 +++
tools/virsh-domain.c |  4 +++
4 files changed, 75 insertions(+)

diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 288e17909..680cd12ff 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -30,4 +30,12 @@ char ** virshDomainNameCompleter(vshControl *ctl,
 const vshCmd *cmd,
 unsigned int flags);

+enum {
+VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC = 1 << 1, /* Return just MACs */


Why are you starting from 2? Why not start from 1?  Didn't you mean 1 << 0?


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

Re: [libvirt] [PATCH 08/18] vshCommandOpt: Allow caller avoiding assert()

[Martin Kletzander]

On Tue, Jan 02, 2018 at 06:12:01PM +0100, Michal Privoznik wrote:

In the future, completer callbacks will receive partially parsed
command (and thus possibly incomplete). However, we still want
them to use command options fetching APIs we already have (e.g.
vshCommandOpt*()) and at the same time don't report any errors
(nor call any asserts).

Signed-off-by: Michal Privoznik 
---
tools/vsh.c | 7 ---
tools/vsh.h | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index ebc8d9cb1..d27acb95b 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
 * to the option if found, 0 with *OPT set to NULL if the name is
 * valid and the option is not required, -1 with *OPT set to NULL if
 * the option is required but not present, and assert if NAME is not
- * valid (which indicates a programming error).  No error messages are
- * issued if a value is returned.
+ * valid (which indicates a programming error) unless cmd->skipChecks
+ * is set. No error messages are issued if a value is returned.
 */
static int
vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
@@ -829,7 +829,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name, 
vshCmdOpt **opt,
/* See if option is valid and/or required.  */
*opt = NULL;
while (valid) {
-assert(valid->name);
+if (!cmd->skipChecks)
+assert(valid->name);


This can segfault when cmd->skipChecks == False && valid->name == NULL,
which is what the assert() guarded before.

So either STREQ_NULLABLE or another if.


if (STREQ(name, valid->name))
break;
valid++;
diff --git a/tools/vsh.h b/tools/vsh.h
index 8f7df9ff8..112b1b57d 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -188,7 +188,8 @@ struct _vshCmdDef {
struct _vshCmd {
const vshCmdDef *def;   /* command definition */
vshCmdOpt *opts;/* list of command arguments */
-vshCmd *next;  /* next command */
+vshCmd *next;   /* next command */
+bool skipChecks;/* skip validity checks when retrieving opts */
};

/*
--
2.13.6

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


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

Re: [libvirt] [PATCH 04/18] vshCommandStringParse: Allow retrieving partial result

[Martin Kletzander]

On Tue, Jan 02, 2018 at 06:11:57PM +0100, Michal Privoznik wrote:

In the future, this function is going to be called from
vshReadlineParse() to provide parsed input for completer
callbacks. The idea is to allow the callbacks to provide more
specific data. For instance, for the following input:

 virsh # domifaddr --domain fedora --interface 

the --interface completer callback is going to be called. Now, it
is more user friendly if the completer offers only those
interfaces found in 'fedora' domain. But in order to do that it
needs to be able to retrieve partially parsed result.

Signed-off-by: Michal Privoznik 
---
tools/virsh.c  |   4 +-
tools/virt-admin.c |   4 +-
tools/vsh.c| 111 +
tools/vsh.h|   2 +-
4 files changed, 82 insertions(+), 39 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 2366b7b71..34eb592ef 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -1519,11 +1544,12 @@ vshCommandParse(vshControl *ctl, vshCommandParser 
*parser)
last->next = arg;
last = arg;

-vshDebug(ctl, VSH_ERR_INFO, "%s: %s(%s): %s\n",
- cmd->name,
- opt->name,
- opt->type != VSH_OT_BOOL ? _("optdata") : _("bool"),
- opt->type != VSH_OT_BOOL ? arg->data : _("(none)"));
+if (ctl)


Don't you mean (!partial) here?  This change looks weird otherwise.

After reading the following patches I see that you call this with ctl == NULL
(from readline).  That makes sense, but there are few more places where ctl is
used even when partial != NULL and that needs to be handled correctly.


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

[libvirt] [PATCH v2] domain_conf: skip boot order check of CD-ROM or floppy device when change-media

[Chen Hanxiao]

From: Chen Hanxiao 

If we insert or eject a CD-ROM/floppy device by:
 'virsh change-media VM --eject/--insert some.iso --live',
and the original CD-ROM device was configed with a boot order,
we may get:
  unsupported configuration: boot order 2 is already used by another device

We just updated 'source file' section rather than hotplug a new device.
This check should be skipped in this case.

Signed-off-by: Chen Hanxiao 
---
v2:
  commit message updated
  remove ATTRIBUTE_UNUSED from @device

 src/conf/domain_conf.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1c25060f..e006cea0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26881,17 +26881,26 @@ virDomainDeviceIsUSB(virDomainDeviceDefPtr dev)
 
 static int
 virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED,
-  virDomainDeviceDefPtr device 
ATTRIBUTE_UNUSED,
+  virDomainDeviceDefPtr device,
   virDomainDeviceInfoPtr info,
   void *opaque)
 {
 virDomainDeviceInfoPtr newinfo = opaque;
+virDomainDiskDefPtr disk = device->data.disk;
+int disk_device = disk->device;
 
 if (info->bootIndex == newinfo->bootIndex) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("boot order %u is already used by another device"),
-   newinfo->bootIndex);
-return -1;
+/* Skip check for insert or eject CD-ROM device */
+if (disk_device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
+disk_device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
+VIR_DEBUG("Skip boot index check for floppy or CDROM");
+return 0;
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("boot order %u is already used by another 
device"),
+   newinfo->bootIndex);
+return -1;
+}
 }
 return 0;
 }
-- 
2.14.3

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

Re: [libvirt] [PATCH 3/3] nodedev: update caps before invoking nodedev driver interfaces

[Erik Skultety]

On Wed, Jan 10, 2018 at 08:14:51PM +0800, Wu Zongyong wrote:
> Some capabilities of node devices rely on what driver they bound to,
> and therefore, these capabilities may change when the driver change.
> So, it is necessary to manually update devices' capabilities each time
> before nodedev driver interfaces invoked.
>
> Signed-off-by: Wu Zongyong 
> ---

Thank you for posting the patch, since I hadn't noticed the problem with other
APIs until I read it.
It was a sad realization that a driver change is not reflected by a udev/kernel
CHANGE event, that would make things much much simpler. I have an idea to either
make this patch shorter or not needed at all though, we'll see when I finish my
patch (it's a long-needed refactor). Despite I haven't found any major flaws in
this patch, let's just put it on hold for a while until I finish my
investigation/work on my patch and see we can really do better here.

Thanks,
Erik

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

[libvirt] [PATCH] virt-aa-helper: Allow parsing supported features for qemu/kvm

[Shivaprasad G Bhat]

The virt-aa-helper fails to parse the xmls with the memory/cpu
hotplug features or user assigned aliases. Set the features in
xmlopt->config for the parsing to succeed.

Signed-off-by: Shivaprasad G Bhat 
---
 src/conf/domain_conf.c|   21 -
 src/conf/domain_conf.h|   21 +
 src/security/virt-aa-helper.c |7 +++
 3 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1c2506..20ce83e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -64,27 +64,6 @@
 
 VIR_LOG_INIT("conf.domain_conf");
 
-/* This structure holds various callbacks and data needed
- * while parsing and creating domain XMLs */
-struct _virDomainXMLOption {
-virObject parent;
-
-/* XML parser callbacks and defaults */
-virDomainDefParserConfig config;
-
-/* domain private data management callbacks */
-virDomainXMLPrivateDataCallbacks privateData;
-
-/* XML namespace callbacks */
-virDomainXMLNamespace ns;
-
-/* ABI stability callbacks */
-virDomainABIStability abi;
-
-/* Private data for save image stored in snapshot XML */
-virSaveCookieCallbacks saveCookie;
-};
-
 #define VIR_DOMAIN_DEF_FORMAT_COMMON_FLAGS \
 (VIR_DOMAIN_DEF_FORMAT_SECURE | \
  VIR_DOMAIN_DEF_FORMAT_INACTIVE | \
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6f7f96b..aacb88a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2662,6 +2662,27 @@ struct _virDomainABIStability {
 virDomainABIStabilityDomain domain;
 };
 
+/* This structure holds various callbacks and data needed
+ * while parsing and creating domain XMLs */
+struct _virDomainXMLOption {
+virObject parent;
+
+/* XML parser callbacks and defaults */
+virDomainDefParserConfig config;
+
+/* domain private data management callbacks */
+virDomainXMLPrivateDataCallbacks privateData;
+
+/* XML namespace callbacks */
+virDomainXMLNamespace ns;
+
+/* ABI stability callbacks */
+virDomainABIStability abi;
+
+/* Private data for save image stored in snapshot XML */
+virSaveCookieCallbacks saveCookie;
+};
+
 virDomainXMLOptionPtr virDomainXMLOptionNew(virDomainDefParserConfigPtr config,
 
virDomainXMLPrivateDataCallbacksPtr priv,
 virDomainXMLNamespacePtr xmlns,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index f7ccae0..8b0ca46 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -699,6 +699,13 @@ get_definition(vahControl * ctl, const char *xmlStr)
 goto exit;
 }
 
+if (virtType == VIR_DOMAIN_VIRT_QEMU || virtType == VIR_DOMAIN_VIRT_KVM) {
+ctl->xmlopt->config.features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG |
+   VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN |
+   VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS 
|
+   VIR_DOMAIN_DEF_FEATURE_USER_ALIAS;
+}
+
 if (virCapabilitiesAddGuestDomain(guest,
   virtType,
   NULL,

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

Re: [libvirt] [PATCH 2/3] nodedev: update mdev_types caps before dumpxml

[Erik Skultety]

On Wed, Jan 10, 2018 at 08:14:50PM +0800, Wu Zongyong wrote:
> In current implemention, mdev_types caps keep constant all
> the time. But, it is possible that a device capable of
> mdev_types sometime(for example:bind to proper driver) and
> incapable of mdev_types at other times(for example: unbind
> from its driver).
> We should keep the info of xml dumped out consistent with
> real status of the device.
>
> Signed-off-by: Wu Zongyong 

For patches 1 and 2 (I'll tune the commit message for this one a bit before
pushing):

Reviewed-by: Erik Skultety 

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

[libvirt] Plan for next release

[Daniel Veillard]

  So first release for the year comes with a renumbering, so 4.0.0 should
come soon, usually we target mid january, so I suggest to enter freeze tomorrow
get an RC2 on Monday and hence have the first release Wed next week.

  I hope this works for everyone,

   thanks


Daniel


-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH 3/3] util: virsysinfo: parse frequency information on S390

[Bjoern Walk]

John Ferlan  [2018-01-10, 05:34PM -0500]:
> Thinking partially in terms of something Andrea posted as a result of
> something I commented on:
> 
> https://www.redhat.com/archives/libvir-list/2018-January/msg00240.html
> 
> Maybe we should take the approach of leaving it as zero if we cannot
> find what we're looking for...
> 
> So the above changes to
> 
> if ((tmp_base = strstr(tmp_base, "cpu MHz dynamic")) &&
> (virSysinfoParseS390Line(tmp_base..., ))
>  = mhz;
> 
> IOW: Update the value if both pass and do nothing if one or the other
> fails.  If both pass then mhz won't be leaked because we'll save it.

Actually, this is not necessary. Also my initial point was irrelevant.
If no frequency information is available, we just completely skip the
whole loop (since no line with "cpu number" is found) and everything
works as before.

So, with my changes right now, we parse liberally, the only way we bail
out with error (and discard everything in the process) is when the
virStrToLong_uip fails. We report a debug message for the unlikely case
of n >= ret->nprocessor.

I have also added a new test case to the sysinfotest with updated data
for /proc/cpuinfo and /proc/sysinfo. Will send v2 shortly.
> 
> BTW: Since Andrea has pushed and patches 1 & 2 were R-B'd, I've pushed
> them.  So it'll just be this third patch with adjustments.

Thanks a lot!

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

Best,
Bjoern

-- 
IBM Systems
Linux on z Systems & Virtualization Development

IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bw...@de.ibm.com

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


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

[libvirt] [PATCH] qemu: Ignore fallback CPU attribute on reconnect

[Jiri Denemark]

When reconnecting to a running domain with host-model CPU started by old
libvirt which did not store the actual CPU in the status XML, we need to
ignore the fallback attribute to make sure we can translate the detected
host CPU model to a model which is supported by the running QEMU.

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

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

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1a0923af36..25ec464d3e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3887,6 +3887,11 @@ qemuProcessUpdateCPU(virQEMUDriverPtr driver,
 virDomainCapsCPUModelsPtr models = NULL;
 int ret = -1;
 
+/* The host CPU model comes from host caps rather than QEMU caps so
+ * fallback must be allowed no matter what the user specified in the XML.
+ */
+vm->def->cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
+
 if (qemuProcessFetchGuestCPU(driver, vm, asyncJob, , ) < 0)
 goto cleanup;
 
-- 
2.15.1

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