[libvirt] [PATCH] docs: Add notes for VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA
When listing snapshot with VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, it
always returns 0 or no snapshot. Because we never implement funtions
to list no-metadata snapshot in virDomainSnapshotObjListGetNames():
if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) ==
VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA)
return 0;
Add notes for that flag.
Please update the comment and man page of that flag when no-metadata
snapshot list is implemented in the future.
Signed-off-by: Han Han
---
include/libvirt/libvirt-domain-snapshot.h | 5 -
tools/virsh.pod | 3 +++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain-snapshot.h
b/include/libvirt/libvirt-domain-snapshot.h
index 20771f9b1e..2e19a52a5c 100644
--- a/include/libvirt/libvirt-domain-snapshot.h
+++ b/include/libvirt/libvirt-domain-snapshot.h
@@ -93,7 +93,10 @@ char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr
snapshot,
* of flag (1<<0) depends on which function it is passed to; but serves
* to toggle the per-call default of whether the listing is shallow or
* recursive. Remaining bits come in groups; if all bits from a group are
- * 0, then that group is not used to filter results. */
+ * 0, then that group is not used to filter results. Internal functions
+ * for listing no-metadata snapshots aren't implemented. Functions above
+ * will return 0 when VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA is used.
+ * */
typedef enum {
VIR_DOMAIN_SNAPSHOT_LIST_ROOTS = (1 << 0), /* Filter by snapshots
with no parents, when
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 86c041d575..b3d3840c2b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4689,6 +4689,9 @@ B of a persistent domain, or be lost on
B of
a transient domain. Likewise, if I<--no-metadata> is specified,
the list will be filtered to just snapshots that exist without
the need for libvirt metadata.
+Note that - It will return no snapshot when I<--no-metadata> is
+used since internal functions for listing no-metadata snapshot
+are not implemented.
If I<--inactive> is specified, the list will be filtered to snapshots
that were taken when the domain was shut off. If I<--active> is
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] libvirt: add daemon itself as shutdown reason
On Tue, Nov 13, 2018 at 03:36:59PM -0500, John Ferlan wrote: > From: Nikolay Shirokovskiy > > This patch introduces a new shutdown reason "daemon" in order > to indicate that the daemon needed to force shutdown the domain > as the best course of action to take at the moment. > > This action would occur during reconnection when processing > encounters an error once the monitor reconnection is successful. > > Signed-off-by: Nikolay Shirokovskiy > Reviewed-by: John Ferlan > --- FWIW: Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] libvirt: add daemon itself as shutdown reason
On 13.11.2018 23:36, John Ferlan wrote: > From: Nikolay Shirokovskiy > > This patch introduces a new shutdown reason "daemon" in order > to indicate that the daemon needed to force shutdown the domain > as the best course of action to take at the moment. > > This action would occur during reconnection when processing > encounters an error once the monitor reconnection is successful. > > Signed-off-by: Nikolay Shirokovskiy > Reviewed-by: John Ferlan > --- > > Since I hadn't seen a yay or nay on my last response related to > changes - figured I'd post things as they are now just to make it > easier to determine whether the changes made were acceptible > This is essentially your original patch merged with my other > changes and the removal of the qemu_process.c change. > > include/libvirt/libvirt-domain.h | 2 ++ > src/conf/domain_conf.c | 3 ++- > src/qemu/qemu_process.c | 6 +- > tools/virsh-domain-monitor.c | 3 ++- > 4 files changed, 11 insertions(+), 3 deletions(-) > I thought you are going to push the patch after last letter) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv8 2/2] tools: Add help docs explaining 'domstats' cache monitor result
Add help document in explaining the cache monitor related 'domstats' result. This patch is written to address John's review comment regarding patch16-v7 and expected to be merged with previous patch and using that patch's committing message. Signed-off-by: Wang Huaqiang --- src/libvirt-domain.c | 21 - tools/virsh.pod | 13 + 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 4895f9f..67ff430 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11345,15 +11345,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "cpu.user" - user cpu time spent in nanoseconds as unsigned long long. * "cpu.system" - system cpu time spent in nanoseconds as unsigned long *long. - * "cpu.cache.monitor.count" - tocal cache monitoring groups - * "cpu.cache.monitor.M.name" - name of cache monitoring group 'M' - * "cpu.cache.monitor.M.vcpus" - vcpus for cache monitoring group 'M' - * "cpu.cache.monitor.M.bank.count" - total bank number of cache monitoring - *group 'M' - * "cpu.cache.monitor.M.bank.N.id" - OS assigned cache bank id for cache - *'N' in cache monitoring group 'M' - * "cpu.cache.monitor.M.bank.N.bytes" - monitor's cache occupancy of cache - *bank 'N' in cache monitoring group 'M' + * "cpu.cache.monitor.count" - number of cache monitors on this domain + * "cpu.cache.monitor..name" - name of cache monitor + * "cpu.cache.monitor..vcpus" - vcpu list of cache monitor + * "cpu.cache.monitor..bank.count" - number of cache banks in cache + *monitor + * "cpu.cache.monitor..bank..id" - host allocatd cache id for + * bank in cache + * monitor + * "cpu.cache.monitor..bank..bytes" - the amount of last level + *cache that domain is + *using on cache bank + *(in Byte) * * VIR_DOMAIN_STATS_BALLOON: * Return memory balloon device information. diff --git a/tools/virsh.pod b/tools/virsh.pod index 86c041d..49d9ab6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1011,6 +1011,19 @@ I<--cpu-total> returns: "cpu.time" - total cpu time spent for this domain in nanoseconds "cpu.user" - user cpu time spent in nanoseconds "cpu.system" - system cpu time spent in nanoseconds + "cpu.cache.monitor.count" - number of cache monitors on this domain + "cpu.cache.monitor..name" - name of cache monitor + "cpu.cache.monitor..vcpus" - vcpu list of cache monitor + "cpu.cache.monitor..bank.count" - number of cache banks in +cache monitor + "cpu.cache.monitor..bank..id" - host allocatd cache id + for bank in + caach monitor + "cpu.cache.monitor..bank..bytes" - the amount of last +level cache that +domain is using on +cache bank +(in Byte) I<--balloon> returns: -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv8 1/2] qemu: Report cache occupancy (CMT) with domstats
Adding the interface in qemu to report CMT statistic information
through command 'virsh domstats --cpu-total'.
Below is a typical output:
# virsh domstats 1 --cpu-total
Domain: 'ubuntu16.04-base'
...
cpu.cache.monitor.count=2
cpu.cache.monitor.0.name=vcpus_1
cpu.cache.monitor.0.vcpus=1
cpu.cache.monitor.0.bank.count=2
cpu.cache.monitor.0.bank.0.id=0
cpu.cache.monitor.0.bank.0.bytes=4505600
cpu.cache.monitor.0.bank.1.id=1
cpu.cache.monitor.0.bank.1.bytes=5586944
cpu.cache.monitor.1.name=vcpus_4-6
cpu.cache.monitor.1.vcpus=4,5,6
cpu.cache.monitor.1.bank.count=2
cpu.cache.monitor.1.bank.0.id=0
cpu.cache.monitor.1.bank.0.bytes=17571840
cpu.cache.monitor.1.bank.1.id=1
cpu.cache.monitor.1.bank.1.bytes=29106176
Signed-off-by: Wang Huaqiang
---
src/libvirt-domain.c | 9 +++
src/qemu/qemu_driver.c | 198 +
2 files changed, 207 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7690339..4895f9f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11345,6 +11345,15 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* "cpu.user" - user cpu time spent in nanoseconds as unsigned long long.
* "cpu.system" - system cpu time spent in nanoseconds as unsigned long
*long.
+ * "cpu.cache.monitor.count" - tocal cache monitoring groups
+ * "cpu.cache.monitor.M.name" - name of cache monitoring group 'M'
+ * "cpu.cache.monitor.M.vcpus" - vcpus for cache monitoring group 'M'
+ * "cpu.cache.monitor.M.bank.count" - total bank number of cache monitoring
+ *group 'M'
+ * "cpu.cache.monitor.M.bank.N.id" - OS assigned cache bank id for cache
+ *'N' in cache monitoring group 'M'
+ * "cpu.cache.monitor.M.bank.N.bytes" - monitor's cache occupancy of cache
+ *bank 'N' in cache monitoring group 'M'
*
* VIR_DOMAIN_STATS_BALLOON:
* Return memory balloon device information.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 89d46ee..d41ae66 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19698,6 +19698,199 @@ typedef enum {
#define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
+typedef struct _virQEMUCpuResMonitorData virQEMUCpuResMonitorData;
+typedef virQEMUCpuResMonitorData *virQEMUCpuResMonitorDataPtr;
+struct _virQEMUCpuResMonitorData{
+const char *name;
+char *vcpus;
+virResctrlMonitorType tag;
+virResctrlMonitorStatsPtr stats;
+size_t nstats;
+};
+
+
+static int
+qemuDomainGetCpuResMonitorData(virDomainObjPtr dom,
+ virQEMUCpuResMonitorDataPtr mondata)
+{
+virDomainResctrlDefPtr resctrl = NULL;
+size_t i = 0;
+size_t j = 0;
+size_t l = 0;
+
+for (i = 0; i < dom->def->nresctrls; i++) {
+resctrl = dom->def->resctrls[i];
+
+for (j = 0; j < resctrl->nmonitors; j++) {
+virDomainResctrlMonDefPtr domresmon = NULL;
+virResctrlMonitorPtr monitor = resctrl->monitors[j]->instance;
+
+domresmon = resctrl->monitors[j];
+mondata[l].tag = domresmon->tag;
+
+/* If virBitmapFormat successfully returns an vcpu string, then
+ * mondata[l].vcpus is assigned with an memory space holding it,
+ * let this newly allocated memory buffer to be freed along with
+ * the free of 'mondata' */
+if (!(mondata[l].vcpus = virBitmapFormat(domresmon->vcpus)))
+return -1;
+
+if (!(mondata[l].name = virResctrlMonitorGetID(monitor))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not get monitor ID"));
+return -1;
+}
+
+if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) {
+if (virResctrlMonitorGetCacheOccupancy(monitor,
+ [l].stats,
+ [l].nstats) < 0)
+return -1;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Invalid CPU resource type"));
+return -1;
+}
+
+l++;
+}
+}
+
+return 0;
+}
+
+
+static int
+qemuDomainGetStatsCpuResMonitorPerTag(virQEMUCpuResMonitorDataPtr mondata,
+ size_t nmondata,
+ virResctrlMonitorType tag,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+unsigned int nmonitors = 0;
+const char *resname =
Re: [libvirt] [PATCHv8 16/17] qemu: Report cache occupancy (CMT) with domstats
> -Original Message-
> From: John Ferlan [mailto:jfer...@redhat.com]
> Sent: Wednesday, November 14, 2018 7:57 AM
> To: Wang, Huaqiang ; libvir-list@redhat.com
> Cc: Feng, Shaohe ; Ding, Jian-feng feng.d...@intel.com>; Zang, Rui
> Subject: Re: [PATCHv8 16/17] qemu: Report cache occupancy (CMT) with
> domstats
>
>
>
> On 11/12/18 8:31 AM, Wang Huaqiang wrote:
> > Adding the interface in qemu to report CMT statistic information
> > through command 'virsh domstats --cpu-total'.
>
> I believe virsh.pod needs an update to list this type of data in the domstats
> --
> cpu-total output.
Thanks for mentioning, I haven't realized virsh.pod need a fix.
I will send out a standalone patch for patch16 and adding this fix soon.
>
> In any case, I need to drop off for the evening - I'll pick this up in the
> morning.
> You can send an update to this patch and I can merge it in.
>
> The first 15 patches look good with some minor adjustments...
>
> John
>
Thanks for your review and all your hard work.
Huaqiang
> >
> > Below is a typical output:
> >
> > # virsh domstats 1 --cpu-total
> > Domain: 'ubuntu16.04-base'
> >...
> >cpu.cache.monitor.count=2
> >cpu.cache.monitor.0.name=vcpus_1
> >cpu.cache.monitor.0.vcpus=1
> >cpu.cache.monitor.0.bank.count=2
> >cpu.cache.monitor.0.bank.0.id=0
> >cpu.cache.monitor.0.bank.0.bytes=4505600
> >cpu.cache.monitor.0.bank.1.id=1
> >cpu.cache.monitor.0.bank.1.bytes=5586944
> >cpu.cache.monitor.1.name=vcpus_4-6
> >cpu.cache.monitor.1.vcpus=4,5,6
> >cpu.cache.monitor.1.bank.count=2
> >cpu.cache.monitor.1.bank.0.id=0
> >cpu.cache.monitor.1.bank.0.bytes=17571840
> >cpu.cache.monitor.1.bank.1.id=1
> >cpu.cache.monitor.1.bank.1.bytes=29106176
> >
> > Signed-off-by: Wang Huaqiang
> > ---
> > src/libvirt-domain.c | 9 +++
> > src/qemu/qemu_driver.c | 198
> > +
> > 2 files changed, 207 insertions(+)
> >
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index
> > 7690339..4895f9f 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -11345,6 +11345,15 @@ virConnectGetDomainCapabilities(virConnectPtr
> conn,
> > * "cpu.user" - user cpu time spent in nanoseconds as unsigned long
> > long.
> > * "cpu.system" - system cpu time spent in nanoseconds as unsigned long
> > *long.
> > + * "cpu.cache.monitor.count" - tocal cache monitoring groups
> > + * "cpu.cache.monitor.M.name" - name of cache monitoring group 'M'
> > + * "cpu.cache.monitor.M.vcpus" - vcpus for cache monitoring group 'M'
> > + * "cpu.cache.monitor.M.bank.count" - total bank number of cache
> monitoring
> > + *group 'M'
> > + * "cpu.cache.monitor.M.bank.N.id" - OS assigned cache bank id for
> > cache
> > + *'N' in cache monitoring group 'M'
> > + * "cpu.cache.monitor.M.bank.N.bytes" - monitor's cache occupancy of
> cache
> > + *bank 'N' in cache monitoring group 'M'
> > *
> > * VIR_DOMAIN_STATS_BALLOON:
> > * Return memory balloon device information.
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
> > 89d46ee..d41ae66 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -19698,6 +19698,199 @@ typedef enum { #define HAVE_JOB(flags)
> > ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
> >
> >
> > +typedef struct _virQEMUCpuResMonitorData virQEMUCpuResMonitorData;
> > +typedef virQEMUCpuResMonitorData *virQEMUCpuResMonitorDataPtr;
> struct
> > +_virQEMUCpuResMonitorData{
>
> Data {
>
> > +const char *name;
> > +char *vcpus;
> > +virResctrlMonitorType tag;
> > +virResctrlMonitorStatsPtr stats;
> > +size_t nstats;
> > +};
> > +
> > +
> > +static int
> > +qemuDomainGetCpuResMonitorData(virDomainObjPtr dom,
> > + virQEMUCpuResMonitorDataPtr mondata) {
> > +virDomainResctrlDefPtr resctrl = NULL;
> > +size_t i = 0;
> > +size_t j = 0;
> > +size_t l = 0;
> > +
> > +for (i = 0; i < dom->def->nresctrls; i++) {
> > +resctrl = dom->def->resctrls[i];
> > +
> > +for (j = 0; j < resctrl->nmonitors; j++) {
> > +virDomainResctrlMonDefPtr domresmon = NULL;
> > +virResctrlMonitorPtr monitor =
> > + resctrl->monitors[j]->instance;
> > +
> > +domresmon = resctrl->monitors[j];
> > +mondata[l].tag = domresmon->tag;
> > +
> > +/* If virBitmapFormat successfully returns an vcpu string, then
> > + * mondata[l].vcpus is assigned with an memory space holding
> > it,
> > + * let this newly allocated memory buffer to be freed along
> > with
> > + * the free of 'mondata' */
> > +if
Re: [libvirt] [PATCH v8 00/14] PCI passthrough support on s390
在 2018/11/13 下午10:35, Andrea Bolognani 写道: On Thu, 2018-11-08 at 19:00 +0800, Yi Min Zhao wrote: Abstract The PCI representation in QEMU has been extended for S390 allowing configuration of zPCI attributes like uid (user-defined identifier) and fid (PCI function identifier). The details can be found here: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html To support the new zPCI feature of the S390 platform, a new element of PCI address is introduced. It has two optional attributes, @uid and @fid. For example: If they are defined by the user, unique values within the guest domain must be used. If they are not specified and the architecture requires them, they are automatically generated with non-conflicting values. zPCI address as an extension of the PCI address are stored in a new structure 'virZPCIDeviceAddress' which is a member of common PCI Address structure. Additionally, two hashtables are used for assignment and reservation of zPCI uid/fid. In support of extending the PCI address, a new PCI address extension flag is introduced. This extension flag allows is not only dedicated for the S390 platform but also other architectures needing certain extensions to PCI address space. I have now provided R-b for the only patch that was still missing it, and as far as I'm concerned the series is ready to be pushed. Thanks very much! Dan, do you have any remaining concerns about the XML syntax, or can I go ahead and push? -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 10/14] conf: Allocate/release 'uid' and 'fid' in PCI address
在 2018/11/13 下午10:29, Andrea Bolognani 写道: On Thu, 2018-11-08 at 19:00 +0800, Yi Min Zhao wrote: This patch adds new functions for reservation, assignment and release to handle the uid/fid. If the uid/fid is defined in the domain XML, they will be reserved directly in the collecting phase. If any of them is not defined, we will find out an available value for them from the zPCI address hashtable, and reserve them. For the hotplug case there might not be a zPCI definition. So allocate and reserve uid/fid the case. Assign if needed and reserve uid/fid for the defined case. Signed-off-by: Yi Min Zhao Reviewed-by: Bjoern Walk Reviewed-by: Boris Fiuczynski --- src/conf/device_conf.c | 16 +++ src/conf/device_conf.h | 3 + src/conf/domain_addr.c | 244 + src/conf/domain_addr.h | 12 ++ src/libvirt_private.syms | 5 + src/qemu/qemu_domain_address.c | 59 +++- 6 files changed, 338 insertions(+), 1 deletion(-) Somehow forgot to give my Reviewed-by: Andrea Bolognani during the previous round of reviews O:-) Thanks! -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 16/17] qemu: Report cache occupancy (CMT) with domstats
On 11/12/18 8:31 AM, Wang Huaqiang wrote:
> Adding the interface in qemu to report CMT statistic information
> through command 'virsh domstats --cpu-total'.
I believe virsh.pod needs an update to list this type of data in the
domstats --cpu-total output.
In any case, I need to drop off for the evening - I'll pick this up in
the morning. You can send an update to this patch and I can merge it in.
The first 15 patches look good with some minor adjustments...
John
>
> Below is a typical output:
>
> # virsh domstats 1 --cpu-total
> Domain: 'ubuntu16.04-base'
>...
>cpu.cache.monitor.count=2
>cpu.cache.monitor.0.name=vcpus_1
>cpu.cache.monitor.0.vcpus=1
>cpu.cache.monitor.0.bank.count=2
>cpu.cache.monitor.0.bank.0.id=0
>cpu.cache.monitor.0.bank.0.bytes=4505600
>cpu.cache.monitor.0.bank.1.id=1
>cpu.cache.monitor.0.bank.1.bytes=5586944
>cpu.cache.monitor.1.name=vcpus_4-6
>cpu.cache.monitor.1.vcpus=4,5,6
>cpu.cache.monitor.1.bank.count=2
>cpu.cache.monitor.1.bank.0.id=0
>cpu.cache.monitor.1.bank.0.bytes=17571840
>cpu.cache.monitor.1.bank.1.id=1
>cpu.cache.monitor.1.bank.1.bytes=29106176
>
> Signed-off-by: Wang Huaqiang
> ---
> src/libvirt-domain.c | 9 +++
> src/qemu/qemu_driver.c | 198
> +
> 2 files changed, 207 insertions(+)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 7690339..4895f9f 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11345,6 +11345,15 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
> * "cpu.user" - user cpu time spent in nanoseconds as unsigned long long.
> * "cpu.system" - system cpu time spent in nanoseconds as unsigned long
> *long.
> + * "cpu.cache.monitor.count" - tocal cache monitoring groups
> + * "cpu.cache.monitor.M.name" - name of cache monitoring group 'M'
> + * "cpu.cache.monitor.M.vcpus" - vcpus for cache monitoring group 'M'
> + * "cpu.cache.monitor.M.bank.count" - total bank number of cache
> monitoring
> + *group 'M'
> + * "cpu.cache.monitor.M.bank.N.id" - OS assigned cache bank id for cache
> + *'N' in cache monitoring group 'M'
> + * "cpu.cache.monitor.M.bank.N.bytes" - monitor's cache occupancy of
> cache
> + *bank 'N' in cache monitoring group 'M'
> *
> * VIR_DOMAIN_STATS_BALLOON:
> * Return memory balloon device information.
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 89d46ee..d41ae66 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -19698,6 +19698,199 @@ typedef enum {
> #define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
>
>
> +typedef struct _virQEMUCpuResMonitorData virQEMUCpuResMonitorData;
> +typedef virQEMUCpuResMonitorData *virQEMUCpuResMonitorDataPtr;
> +struct _virQEMUCpuResMonitorData{
Data {
> +const char *name;
> +char *vcpus;
> +virResctrlMonitorType tag;
> +virResctrlMonitorStatsPtr stats;
> +size_t nstats;
> +};
> +
> +
> +static int
> +qemuDomainGetCpuResMonitorData(virDomainObjPtr dom,
> + virQEMUCpuResMonitorDataPtr mondata)
> +{
> +virDomainResctrlDefPtr resctrl = NULL;
> +size_t i = 0;
> +size_t j = 0;
> +size_t l = 0;
> +
> +for (i = 0; i < dom->def->nresctrls; i++) {
> +resctrl = dom->def->resctrls[i];
> +
> +for (j = 0; j < resctrl->nmonitors; j++) {
> +virDomainResctrlMonDefPtr domresmon = NULL;
> +virResctrlMonitorPtr monitor = resctrl->monitors[j]->instance;
> +
> +domresmon = resctrl->monitors[j];
> +mondata[l].tag = domresmon->tag;
> +
> +/* If virBitmapFormat successfully returns an vcpu string, then
> + * mondata[l].vcpus is assigned with an memory space holding it,
> + * let this newly allocated memory buffer to be freed along with
> + * the free of 'mondata' */
> +if (!(mondata[l].vcpus = virBitmapFormat(domresmon->vcpus)))
> +return -1;
> +
> +if (!(mondata[l].name = virResctrlMonitorGetID(monitor))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not get monitor ID"));
> +return -1;
> +}
> +
> +if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) {
> +if (virResctrlMonitorGetCacheOccupancy(monitor,
> + [l].stats,
> + [l].nstats) <
> 0)
> +return -1;
> +} else {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +
Re: [libvirt] [PATCHv8 15/17] qemu: Refactor qemuDomainGetStatsCpu
On 11/12/18 8:31 AM, Wang Huaqiang wrote:
> Refactoring qemuDomainGetStatsCpu, make it possible to add
> more CPU statistics.
>
> Signed-off-by: Wang Huaqiang
> ---
> src/qemu/qemu_driver.c | 21 -
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 09e04b8..89d46ee 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -19699,11 +19699,9 @@ typedef enum {
>
>
> static int
> -qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> - virDomainObjPtr dom,
> - virDomainStatsRecordPtr record,
> - int *maxparams,
> - unsigned int privflags ATTRIBUTE_UNUSED)
> +qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
> +virDomainStatsRecordPtr record,
> +int *maxparams)
> {
> qemuDomainObjPrivatePtr priv = dom->privateData;
> unsigned long long cpu_time = 0;
> @@ -19739,6 +19737,19 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver
> ATTRIBUTE_UNUSED,
> return 0;
> }
>
> +
> +static int
> +qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> + virDomainObjPtr dom,
> + virDomainStatsRecordPtr record,
> + int *maxparams,
> + unsigned int privflags ATTRIBUTE_UNUSED)
> +{
> +if (qemuDomainGetStatsCpuCgroup(dom, record, maxparams) < 0)
> +return -1;
This should just be:
return qemuDomainGetStatsCpuCgroup(dom, record, maxparams);
yes, causes a merge conflict in next patch, but that one at least has
the return 0 this one would have needed...
Reviewed-by: John Ferlan
John
> +}
> +
> +
> static int
> qemuDomainGetStatsBalloon(virQEMUDriverPtr driver,
>virDomainObjPtr dom,
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 14/17] qemu: enable resctrl monitor in qemu
On 11/12/18 8:31 AM, Wang Huaqiang wrote: > Add functions for creating, destroying, reconnecting resctrl > monitor in qemu according to the configuration in domain XML. > > Signed-off-by: Wang Huaqiang > --- > src/qemu/qemu_process.c | 49 > - > 1 file changed, 48 insertions(+), 1 deletion(-) > Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 11/17] util: Add more interfaces for resctrl monitor
On 11/12/18 8:31 AM, Wang Huaqiang wrote: > Add interfaces monitor group to support operations such > as add PID, get ID, remove group ... etc. virResctrlMonitorGetStats is perhaps a bit more than just basic stuff! So I can add: Implement the virResctrlMonitorGetStats to fetch all the statistical data and the virResctrlMonitorGetCacheOccupancy in order to fetch the cache specific "llc_oocupancy" value. > > Signed-off-by: Wang Huaqiang > --- > src/libvirt_private.syms | 5 ++ > src/util/virresctrl.c| 167 > +++ > src/util/virresctrl.h| 26 > 3 files changed, 198 insertions(+) > Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 10/17] util: Add interface for setting monitor ID.
On 11/12/18 8:31 AM, Wang Huaqiang wrote:
> Add virResctrlMonitorSetID by leveraging previous refactored patch.
>
> Signed-off-by: Wang Huaqiang
> ---
> src/util/virresctrl.c | 9 +
> 1 file changed, 9 insertions(+)
>
This one won't compile alone - it's missing the virresctrl.h and the
libvirt_private.syms change.
To make life easier - I'm going to merge it with the next patch.
Reviewed-by: John Ferlan
John
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 4e4831c..ed682c9 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -2581,3 +2581,12 @@ virResctrlMonitorCreate(virResctrlMonitorPtr monitor,
> virResctrlUnlock(lockfd);
> return ret;
> }
> +
> +
> +int
> +virResctrlMonitorSetID(virResctrlMonitorPtr monitor,
> + const char *id)
> +
> +{
> +return virResctrlSetID(>id, id);
> +}
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 09/17] util: Refactor virResctrlAllocSetID to set allocation ID
On 11/12/18 8:31 AM, Wang Huaqiang wrote: > Refactor virResctrlAllocSetID. > In new code the error of id overwriting is reported promptly. > > Signed-off-by: Wang Huaqiang > --- > src/util/virresctrl.c | 25 - > 1 file changed, 20 insertions(+), 5 deletions(-) > Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 12/17] conf: Remove virDomainResctrlAppend and introduce virDomainResctrlNew
On 11/12/18 8:31 AM, Wang Huaqiang wrote: > Introduced virDomainResctrlNew to do the most part of virDomainResctrlAppend > and move the operation of appending resctrl to @def->resctrls out of > function. > > Rather than rely on virDomainResctrlAppend to perform the allocation, move > the onus to the caller and make use of virBitmapNewCopy for @vcpus and > virObjectRef for @alloc, thus removing the need to set each to NULL after the > call. > > Signed-off-by: Wang Huaqiang > --- > src/conf/domain_conf.c | 56 > +- > 1 file changed, 33 insertions(+), 23 deletions(-) > Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 01/17] docs, util: Refactor schemas and virresctrl to support optional cache
On 11/12/18 8:31 AM, Wang Huaqiang wrote:
> Refactor schemas and virresctrl to support optional element
> in .
>
> Later, the monitor entry will be introduced and to be placed
> under . Either cache entry or monitor entry is
> an optional element of .
>
> An cachetune has no element is taking the default resource
> allocating policy defined in '/sys/fs/resctrl/schemata'.
>
> Signed-off-by: Wang Huaqiang
> ---
> docs/formatdomain.html.in | 4 ++--
> docs/schemas/domaincommon.rng | 4 ++--
> src/util/virresctrl.c | 27 +++
> 3 files changed, 31 insertions(+), 4 deletions(-)
>
[...]
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 5d811a2..7339e9b 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
[...]
> @@ -2302,6 +2320,11 @@ virResctrlAllocAddPID(virResctrlAllocPtr alloc,
> char *pidstr = NULL;
> int ret = 0;
>
> +/* If allocation is empty, then no resctrl directory and the 'tasks' file
> + * exists, just return */
/* If the allocation is empty, then it is impossible to add a PID to
* allocation due to lacking of its 'tasks' file so just return */
> +if (virResctrlAllocIsEmpty(alloc))
> +return 0;
> +
> if (!alloc->path) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot add pid to non-existing resctrl
> allocation"));
> @@ -2334,6 +2357,10 @@ virResctrlAllocRemove(virResctrlAllocPtr alloc)
> {
> int ret = 0;
>
> +/* Do not destroy if path is the system/default path for the allocation
> */
> +if (STREQ(alloc->path, SYSFS_RESCTRL_PATH))
> +return 0;
> +
You added this backwards, must go below the following:
> if (!alloc->path)
> return 0;
>
>
With those changes,
Reviewed-by: John Ferlan
(As with the rest, I can make the changes for you).
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 04/17] util: Add interface to determine monitor path
On 11/12/18 8:31 AM, Wang Huaqiang wrote:
> Add interface for resctrl monitor to determine the path.
>
> Signed-off-by: Wang Huaqiang
>
> Reviewed-by: John Ferlan
> ---
> src/libvirt_private.syms | 1 +
> src/util/virresctrl.c| 55
>
> src/util/virresctrl.h| 5 -
> 3 files changed, 60 insertions(+), 1 deletion(-)
>
[...]
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 463555c..aa062c3 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
[...]
> +int
> +virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor,
> + const char *machinename)
> +{
> +VIR_AUTOFREE(char *) parentpath = NULL;
> +
> +if (!monitor) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Invalid resctrl monitor"));
> +return -1;
> +}
In the v7 in a follow-up to patch 4 we agreed the following check was fine:
if (!monitor->alloc) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing resctrl monitor alloc"));
return -1;
}
> +
> +if (monitor->path) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Resctrl monitor path is already set to '%s'"),
> + monitor->path);
> +return -1;
> +}
> +
> +if (monitor->id && monitor->alloc && monitor->alloc->id) {
> +if (STREQ(monitor->id, monitor->alloc->id)) {
> +if (VIR_STRDUP(monitor->path, monitor->alloc->path) < 0)
> +return -1;
> +return 0;
> +}
> +}
Changing the above to
if (STREQ_NULLABLE(monitor->id, monitor->alloc->id)) {
if (VIR_STRDUP(monitor->path, monitor->alloc->path) < 0)
return -1;
return 0;
With that,
Reviewed-by: John Ferlan
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 03/17] util: Refactor code for determining allocation path
On 11/12/18 8:31 AM, Wang Huaqiang wrote: > The code for determining resctrl allocation path could be reused > for monitor. Refactor it for reuse. > > Signed-off-by: Wang Huaqiang > --- > src/util/virresctrl.c | 38 ++ > 1 file changed, 30 insertions(+), 8 deletions(-) > Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 05/17] util: Refactor code for adding PID to the resource group
On 11/12/18 8:31 AM, Wang Huaqiang wrote: > The code of adding PID to the allocation could be reused, refactor it > for later reuse. > > Signed-off-by: Wang Huaqiang > --- > src/util/virresctrl.c | 30 +++--- > 1 file changed, 19 insertions(+), 11 deletions(-) Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv8 02/17] util: Introduce resctrl monitor for CMT
On 11/12/18 8:31 AM, Wang Huaqiang wrote: > Cache Monitoring Technology (aka CMT) provides the capability > to report cache utilization information of system task. > > This patch introduces the concept of resctrl monitor through > data structure virResctrlMonitor. > > Signed-off-by: Wang Huaqiang > --- > src/libvirt_private.syms | 1 + > src/util/virresctrl.c| 80 > > src/util/virresctrl.h| 9 ++ > 3 files changed, 84 insertions(+), 6 deletions(-) > [...] > > # util/virrotatingfile.h > diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c > index 7339e9b..e3c84a3 100644 > --- a/src/util/virresctrl.c > +++ b/src/util/virresctrl.c [...] > + * > + * =Cache monitoring technology (CMT)= > + * > + * Cache monitoring technology is used to perceive how many cache the process > + * is using actually. virResctrlMonitor represents the resource control > + * monitoring group, it is supported to monitor resource utilization > + * information on granularity of vcpu. > + * > + * From hardware perspective, cache monitoring technology (CMT), memory >From a hardware > + * bandwidth technology (MBM), as well as the CAT and MBA, are all orthogonal > + * features. The monitor will be created under the scope of default resctrl > + * group if no specific CAT or MBA entries are provided for the guest." > */ Reviewed-by: John Ferlan John [...] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 03/13] security: Always spawn process for transactions
[...] >> >> I understand (generically) why we need the lock. I'm OK with it being >> enabled by default. That's not the question/ask. Building in a way to >> allow disabling usage of virFork and/or metadata lock knowing the >> "penalty" or downside to doing so goes beyond bug free or performance, >> it's just that "choice" we allow someone to make. You know there are >> those out there that will bemoan "choosing" this is as the default. If >> they want to disable in order to gain whatever at the cost of something >> else, then so be it. In some ways it's a CYA exercise. > > Just an idea that I got, what if there won't be any config knob but this > would use namespaces? I mean, if namespaces are on then metadata locking > is happening and if they are off then no metadata locking is happening. > > Since namespaces do mean extra fork(), doing things this way there won't > be any extra fork() if namespaces are off. > I'd prefer to not make metadata locking (files) rely on namespaces (devices). I get the relationship though. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] libvirt: add daemon itself as shutdown reason
From: Nikolay Shirokovskiy
This patch introduces a new shutdown reason "daemon" in order
to indicate that the daemon needed to force shutdown the domain
as the best course of action to take at the moment.
This action would occur during reconnection when processing
encounters an error once the monitor reconnection is successful.
Signed-off-by: Nikolay Shirokovskiy
Reviewed-by: John Ferlan
---
Since I hadn't seen a yay or nay on my last response related to
changes - figured I'd post things as they are now just to make it
easier to determine whether the changes made were acceptible
This is essentially your original patch merged with my other
changes and the removal of the qemu_process.c change.
include/libvirt/libvirt-domain.h | 2 ++
src/conf/domain_conf.c | 3 ++-
src/qemu/qemu_process.c | 6 +-
tools/virsh-domain-monitor.c | 3 ++-
4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index fdd2d6b8ea..71debd92b8 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -145,6 +145,8 @@ typedef enum {
VIR_DOMAIN_SHUTOFF_FAILED = 6, /* domain failed to start */
VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT = 7, /* restored from a snapshot which was
* taken while domain was shutoff */
+VIR_DOMAIN_SHUTOFF_DAEMON = 8, /* daemon decides to kill domain
+ during reconnection processing */
# ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_SHUTOFF_LAST
# endif
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e8e0adc819..b45f25fabb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -779,7 +779,8 @@ VIR_ENUM_IMPL(virDomainShutoffReason,
VIR_DOMAIN_SHUTOFF_LAST,
"migrated",
"saved",
"failed",
- "from snapshot")
+ "from snapshot",
+ "daemon")
VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST,
"unknown",
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1850923914..59a94bb74a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7978,10 +7978,14 @@ qemuProcessReconnect(void *opaque)
*
* If we cannot get to the monitor when the QEMU command
* line used -no-shutdown, then we can safely say that the
- * domain crashed; otherwise, we don't really know. */
+ * domain crashed; otherwise, if the monitor was started,
+ * then we can blame ourselves, else we failed before the
+ * monitor started so we don't really know. */
if (!priv->mon && tryMonReconn &&
qemuDomainIsUsingNoShutdown(priv))
state = VIR_DOMAIN_SHUTOFF_CRASHED;
+else if (priv->mon)
+state = VIR_DOMAIN_SHUTOFF_DAEMON;
else
state = VIR_DOMAIN_SHUTOFF_UNKNOWN;
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 3a2636377d..f0ad558f87 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -212,7 +212,8 @@ VIR_ENUM_IMPL(virshDomainShutoffReason,
N_("migrated"),
N_("saved"),
N_("failed"),
- N_("from snapshot"))
+ N_("from snapshot"),
+ N_("daemon"))
VIR_ENUM_DECL(virshDomainCrashedReason)
VIR_ENUM_IMPL(virshDomainCrashedReason,
--
2.17.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 3/3] qemu: Add ability to invalidate the capabilities cache
Provide a mechanism via the QEMU driver reload functionality
to invalidate all the capabilities cache and force a reread
of the capabilities rather than requiring an admin to "know"
they need to remove all the capability cache files and restart
libvirtd. This still requires usage of the reload functionality,
but it provides the "internal mechanism" in order to cause a
reread rather than the "external need" to remove and restart.
Signed-off-by: John Ferlan
---
src/qemu/qemu_capabilities.c | 35 +++
src/qemu/qemu_capabilities.h | 2 ++
src/qemu/qemu_driver.c | 4 +++-
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ebe0c0c7df..6b66ee006c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -553,6 +553,7 @@ struct _virQEMUCaps {
bool usedQMP;
bool isNested;
+bool invalid;
char *binary;
time_t ctime;
@@ -3870,6 +3871,11 @@ virQEMUCapsIsValid(void *data,
if (!qemuCaps->binary)
return true;
+if (qemuCaps->invalid) {
+VIR_DEBUG("capabilities for '%s' invalidated", qemuCaps->binary);
+return false;
+}
+
if (qemuCaps->libvirtCtime != virGetSelfLastChanged() ||
qemuCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) {
VIR_DEBUG("Outdated capabilities for '%s': libvirt changed "
@@ -4958,6 +4964,35 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
return ret;
}
+
+/** virQEMUCapsInvalidateCapabilities:
+ *
+ * Using the @driver and existing qemuCapsCache, force all the data
+ * in the cache to be invalidated so that a subsequent isValid call
+ * force a refetch of the capabilities data.
+ */
+
+static int
+virQEMUCapsInvalidateData(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+virQEMUCaps *qemuCaps = payload;
+
+qemuCaps->invalid = true;
+
+return 0;
+}
+
+
+void
+virQEMUCapsInvalidateCapabilities(virFileCachePtr cache)
+{
+virFileCacheForEach(cache, virQEMUCapsInvalidateData, NULL);
+return;
+}
+
+
bool
virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
const virDomainDef *def)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6bb9a2c8f0..0600ecd424 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -598,6 +598,8 @@ virCapsPtr virQEMUCapsInit(virFileCachePtr cache);
int virQEMUCapsGetDefaultVersion(virCapsPtr caps,
virFileCachePtr capsCache,
unsigned int *version);
+void
+virQEMUCapsInvalidateCapabilities(virFileCachePtr cache);
VIR_ENUM_DECL(virQEMUCaps);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 09e04b8544..3b6b399e5b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -968,7 +968,9 @@ qemuStateReload(void)
if (!qemu_driver)
return 0;
-if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false)))
+virQEMUCapsInvalidateCapabilities(qemu_driver->qemuCapsCache);
+
+if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, true)))
goto cleanup;
cfg = virQEMUDriverGetConfig(qemu_driver);
--
2.17.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 0/3] Add mechanisms to force QEMU capabilities refetches
Sending as an RFC primarily because I'm looking for whether either or both mechanisms in the series is more or less desired. Likewise, if it's felt that the current process of telling customers to just delete the cache is acceptible, then so be it. If there's other ideas I'm willing to give them a go too. I did consider adding a virsh option to "virsh capabilities" (still possible) and/or a virt-admin option to force the refresh. These just were "easier" and didn't require an API adjustment to implement. Patch1 is essentially a means to determine if the kernel config was changed to allow nested virtualization and to force a refresh of the capabilities in that case. Without doing so the CPU settings for a guest may not add the vmx=on depending on configuration and for the user that doesn't make sense. There is a private bz on this so I won't bother posting it. Patch2 and Patch3 make use of the 'service libvirtd reload' function in order to invalidate all the entries in the internal QEMU capabilities hash table and then to force a reread. This perhaps has downsides related to guest usage and previous means to use reload and not refresh if a guest was running. On the other hand, we tell people to just clear the QEMU capabilities cache (e.g. rm /var/cache/libvirt/qemu/capabilities/*.xml) and restart libvirtd, so in essence, the same result. It's not clear how frequently this is used (it's essentially a SIGHUP to libvirtd). BTW: I did try deleting the files in the cache in one iteration, but for some reason (and I didn't investigate too long), the files wouldn't be recreated even though the cache was repopulated. They were recreated after a libvirt restart... John Ferlan (3): qemu: Add check for whether nesting was enabled util: Introduce virFileCacheForEach qemu: Add ability to invalidate the capabilities cache src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 78 src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_capspriv.h | 2 + src/qemu/qemu_driver.c | 4 +- src/util/virfilecache.c | 24 +++ src/util/virfilecache.h | 5 +++ tests/qemucapabilitiestest.c | 3 ++ 8 files changed, 118 insertions(+), 1 deletion(-) -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 2/3] util: Introduce virFileCacheForEach
Add an API to allow traversing each of the file cache elements
and invoking a virHashIterator callback function for each element.
Signed-off-by: John Ferlan
---
src/libvirt_private.syms | 1 +
src/util/virfilecache.c | 24
src/util/virfilecache.h | 5 +
3 files changed, 30 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2343a757c1..d59126c83f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1882,6 +1882,7 @@ virFindFileInPath;
# util/virfilecache.h
+virFileCacheForEach;
virFileCacheGetPriv;
virFileCacheInsertData;
virFileCacheLookup;
diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index 15c0d99fd9..cb7580e11a 100644
--- a/src/util/virfilecache.c
+++ b/src/util/virfilecache.c
@@ -349,6 +349,30 @@ virFileCacheLookupByFunc(virFileCachePtr cache,
}
+/**
+ * virFileCacheForEach
+ * @cache: existing cache object
+ * @iter: an iterator to process each data
+ * @opaque: opaque data to pass to the iterator
+ *
+ * For each element in the file cache, run the @iter which uses
+ * @virHashIterator arguments with @opaque data.
+ */
+int
+virFileCacheForEach(virFileCachePtr cache,
+virHashIterator iter,
+void *opaque)
+{
+int ret;
+
+virObjectLock(cache);
+ret = virHashForEach(cache->table, iter, opaque);
+virObjectUnlock(cache);
+
+return ret;
+}
+
+
/**
* virFileCacheGetPriv:
* @cache: existing cache object
diff --git a/src/util/virfilecache.h b/src/util/virfilecache.h
index af6a189d7f..44b766153e 100644
--- a/src/util/virfilecache.h
+++ b/src/util/virfilecache.h
@@ -122,6 +122,11 @@ virFileCacheLookupByFunc(virFileCachePtr cache,
virHashSearcher iter,
const void *iterData);
+int
+virFileCacheForEach(virFileCachePtr cache,
+virHashIterator iter,
+void *opaque);
+
void *
virFileCacheGetPriv(virFileCachePtr cache);
--
2.17.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 1/3] qemu: Add check for whether nesting was enabled
Support for nested kvm is handled via a kernel module configuration
adjustment which if done after libvirtd is started and/or the last
QEMU capabilities adjustment can result in the inability to start a
guest and use nested kvm until the capabilities cache is invalidated.
Thus, let's fetch and save the setting during initialization and then
when the capabilities are checked for various host related adjustments
that could affect whether the capabilities cache is updated add a check
whether the nested value was set for either kvm_intel or kvm_amd to
force a refetch of the capabilities.
Signed-off-by: John Ferlan
---
src/qemu/qemu_capabilities.c | 43
src/qemu/qemu_capspriv.h | 2 ++
tests/qemucapabilitiestest.c | 3 +++
3 files changed, 48 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ca5af3297..ebe0c0c7df 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -40,6 +40,7 @@
#include "virnodesuspend.h"
#include "virnuma.h"
#include "virhostcpu.h"
+#include "virkmod.h"
#include "qemu_monitor.h"
#include "virstring.h"
#include "qemu_hostdev.h"
@@ -551,6 +552,7 @@ struct _virQEMUCaps {
virObject parent;
bool usedQMP;
+bool isNested;
char *binary;
time_t ctime;
@@ -1512,6 +1514,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
return NULL;
ret->usedQMP = qemuCaps->usedQMP;
+ret->isNested = qemuCaps->isNested;
if (VIR_STRDUP(ret->binary, qemuCaps->binary) < 0)
goto error;
@@ -3567,6 +3570,9 @@ virQEMUCapsLoadCache(virArch hostArch,
virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
+qemuCaps->isNested = virXPathBoolean("count(./isNested) > 0",
+ ctxt) > 0;
+
ret = 0;
cleanup:
VIR_FREE(str);
@@ -3786,6 +3792,9 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps)
if (qemuCaps->sevCapabilities)
virQEMUCapsFormatSEVInfo(qemuCaps, );
+if (qemuCaps->isNested)
+virBufferAddLit(, "\n");
+
virBufferAdjustIndent(, -2);
virBufferAddLit(, "\n");
@@ -3826,6 +3835,28 @@ virQEMUCapsSaveFile(void *data,
}
+static bool
+virQEMUCapsIsNested(void)
+{
+VIR_AUTOFREE(char *) kConfig = NULL;
+
+if ((kConfig = virKModConfig()) &&
+(strstr(kConfig, "kvm_intel nested=1") ||
+ strstr(kConfig, "kvm_amd nested=1")))
+return true;
+return false;
+}
+
+
+void
+virQEMUCapsClearIsNested(virQEMUCapsPtr qemuCaps)
+{
+/* For qemucapabilitiestest to avoid printing the on
+ * hosts with nested set in the kernel */
+qemuCaps->isNested = false;
+}
+
+
static bool
virQEMUCapsIsValid(void *data,
void *privData)
@@ -3834,6 +3865,7 @@ virQEMUCapsIsValid(void *data,
virQEMUCapsCachePrivPtr priv = privData;
bool kvmUsable;
struct stat sb;
+bool isNested;
if (!qemuCaps->binary)
return true;
@@ -3866,6 +3898,15 @@ virQEMUCapsIsValid(void *data,
return false;
}
+/* Check if someone changed the nested={0|1} value for the kernel from
+ * the previous time we checked. If so, then refresh the capabilities. */
+isNested = virQEMUCapsIsNested();
+if (isNested != qemuCaps->isNested) {
+VIR_WARN("changed kernel nested kvm value was %d", qemuCaps->isNested);
+qemuCaps->isNested = isNested;
+return false;
+}
+
if (!virQEMUCapsGuestIsNative(priv->hostArch, qemuCaps->arch)) {
VIR_DEBUG("Guest arch (%s) is not native to host arch (%s), "
"skipping KVM-related checks",
@@ -4452,6 +4493,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsInitQMPMonitor(qemuCaps, cmd->mon) < 0)
goto cleanup;
+qemuCaps->isNested = virQEMUCapsIsNested();
+
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
virQEMUCapsInitQMPCommandAbort(cmd);
if ((rc = virQEMUCapsInitQMPCommandRun(cmd, true)) != 0) {
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index 8d1a40fe74..b5d6aae2e5 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -48,6 +48,8 @@ int
virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon);
+void virQEMUCapsClearIsNested(virQEMUCapsPtr qemuCaps);
+
int
virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon);
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index 8fe5a55e1d..703fb6a125 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -63,6 +63,9 @@ testQemuCaps(const void *opaque)
qemuMonitorTestGetMonitor(mon)) < 0)
goto cleanup;
+/* Don't apply what the host has... force clear for testing purposes */
+
Re: [libvirt] [PATCH 2/2] tests: Augment vcgrouptest to add virCgroupGetMemoryStat
On 11/13/18 8:01 AM, Pavel Hrdina wrote:
> On Wed, Nov 07, 2018 at 06:57:40PM -0500, John Ferlan wrote:
>> Add a test to fetch the GetMemoryStat output. This only gets
>> data for v1 only right now since the v2 data from commit 61ff6021
>> is rather useless returning all 0's.
>>
>> Signed-off-by: John Ferlan
>> ---
>> tests/vircgrouptest.c | 61 +++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
>> index 310e1fb6a2..06c4a8ef5c 100644
>> --- a/tests/vircgrouptest.c
>> +++ b/tests/vircgrouptest.c
>> @@ -802,6 +802,64 @@ static int testCgroupGetMemoryUsage(const void *args
>> ATTRIBUTE_UNUSED)
>> return ret;
>> }
>>
>> +
>> +static int
>> +testCgroupGetMemoryStat(const void *args ATTRIBUTE_UNUSED)
>> +{
>> +virCgroupPtr cgroup = NULL;
>> +int rv, ret = -1;
>
> Please each variable on separate line. Once you need to change/remove
> any of the variable the diff is way better.
>
Right - just some copy-pasta here.
>> +size_t i;
>> +
>> +const unsigned long long expected_values[] = {
>> +1336619008ULL,
>> +67100672ULL,
>> +145887232ULL,
>> +661872640ULL,
>> +627400704UL,
>> +3690496ULL
>> +};
>> +const char* names[] = {
>> +"cache",
>> +"active_anon",
>> +"inactive_anon",
>> +"active_file",
>> +"inactive_file",
>> +"unevictable"
>> +};
>> +unsigned long long values[ARRAY_CARDINALITY(expected_values)];
>> +
>> +if ((rv = virCgroupNewPartition("/virtualmachines", true,
>> +(1 << VIR_CGROUP_CONTROLLER_MEMORY),
>> +)) < 0) {
>> +fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
>> -rv);
>> +goto cleanup;
>> +}
>> +
>> +if ((rv = virCgroupGetMemoryStat(cgroup, [0],
>> + [1], [2],
>> + [3], [4],
>> + [5])) < 0) {
>> +fprintf(stderr, "Could not retrieve GetMemoryStat for
>> /virtualmachines cgroup: %d\n", -rv);
>> +goto cleanup;
>> +}
>> +
>> +for (i = 0; i < ARRAY_CARDINALITY(expected_values); i++) {
>> +if (expected_values[i] != (values[i] << 10)) {
>
> This feels wrong and it's just a lucky coincidence that it works with
> these values. It's basically the same operation as 'x * 1024'.
>
> I would rather change it into this:
>
> if ((expected_values[i] >> 10) != values[i]) {
>
> because we know that we do the same operation after reading these values
> from memory.stat file.
>
That's fine - either/or. I forgot to note the values were "sourced
from" the original commit d14524701 MAKE_FILE mocking logic and the
fetch/store logic in virCgroupGetMemoryStat which does the >> 10.
>> +fprintf(stderr,
>> +"Wrong value (%llu) for %s from virCgroupGetMemoryStat
>> (expected %llu)\n",
>> +values[i], names[i], expected_values[i]);
>
> This would print wrong values, we need to print shifted values.
> Probably the best solution would be to have "expected_values" with the
> correct number from the start
Oh yeah - forgot to do that after I realized the >> was necessary... Off
by a magnitude of 1024 is always easy to figure out though. Still the
"correct number" could be a matter of opinion, too. Do you view the
expected number as seen in the array without the shift or with it? e.g.
for 'cache' is 1336619008 (expected w/o shift) or 1305292 (value w/
shift) the correct value?
>
> Note: please keep the lines under 80 characters.
Hey, that's my line ;-)
>
> Because it's a test I'm OK with both solutions, modifying
> "expected_values" in place or to have them correct from the start and
> I'll leave it up to you. There is no need to resend it.
>
> Reviewed-by: Pavel Hrdina
>
Thanks I went with displaying the shifted value:
fprintf(stderr,
"Wrong value (%llu) for %s from virCgroupGetMemoryStat "
"(expected %llu)\n",
values[i], names[i], (expected_values[i] >> 10));
But I won't push right away just in case someone prefers the unshifted
from the expected array.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Fix uninstall when WITH_APPARMOR_PROFILES is defined
On 11/13/18 10:14 AM, Andrea Bolognani wrote: On Wed, 2018-11-07 at 17:44 -0700, Jim Fehlig wrote: [...] @@ -96,6 +96,10 @@ install-apparmor-local: 'usr.lib.libvirt.virt-aa-helper'" \ >$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper Pre-existing lack of quoting... I've fixed it with a trivial patch already. Thanks! +uninstall-apparmor-local: + rm -f "$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" + rmdir $(APPARMOR_LOCAL_DIR) || : Missing quotes here as well. Once you fix that, Reviewed-by: Andrea Bolognani I've fixed it and pushed. While doing so I noticed a lot of pre-existing lack of quoting throughout the various Makefile.am :-). I'm not sure these are worth fixing given the lack problem reports... Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] conf: qemu: add support for Hyper-V PV IPIs
Qemu-3.1 supports Hyper-V-style PV IPIs making it cheaper for Windows
guests to send an IPI, especially when it targets many CPUs.
Signed-off-by: Vitaly Kuznetsov
---
docs/formatdomain.html.in | 7 +++
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c | 6 +-
src/conf/domain_conf.h | 1 +
src/cpu/cpu_x86.c | 3 +++
src/cpu/cpu_x86_data.h | 1 +
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_parse_command.c | 1 +
src/qemu/qemu_process.c | 1 +
tests/qemuxml2argvdata/hyperv-off.xml | 1 +
tests/qemuxml2argvdata/hyperv.args | 2 +-
tests/qemuxml2argvdata/hyperv.xml | 1 +
tests/qemuxml2xmloutdata/hyperv-off.xml | 1 +
tests/qemuxml2xmloutdata/hyperv.xml | 1 +
14 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 269741a690..24d4b68d7b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1981,6 +1981,7 @@
frequencies state='on'/
reenlightenment state='on'/
tlbflush state='on'/
+ipi state='on'/
/hyperv
kvm
hidden state='on'/
@@ -2121,6 +2122,12 @@
on, off
4.7.0 (QEMU 3.0)
+
+ ipi
+ Enable PV IPI support
+ on, off
+ 4.10.0 (QEMU 3.1)
+
pvspinlock
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b9ac5df479..26019b3279 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5769,6 +5769,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e8e0adc819..c8cde90808 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -172,7 +172,8 @@ VIR_ENUM_IMPL(virDomainHyperv, VIR_DOMAIN_HYPERV_LAST,
"vendor_id",
"frequencies",
"reenlightenment",
- "tlbflush")
+ "tlbflush",
+ "ipi")
VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST,
"hidden")
@@ -19990,6 +19991,7 @@ virDomainDefParseXML(xmlDocPtr xml,
case VIR_DOMAIN_HYPERV_FREQUENCIES:
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
case VIR_DOMAIN_HYPERV_TLBFLUSH:
+case VIR_DOMAIN_HYPERV_IPI:
break;
case VIR_DOMAIN_HYPERV_SPINLOCKS:
@@ -22184,6 +22186,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr
src,
case VIR_DOMAIN_HYPERV_FREQUENCIES:
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
case VIR_DOMAIN_HYPERV_TLBFLUSH:
+case VIR_DOMAIN_HYPERV_IPI:
if (src->hyperv_features[i] != dst->hyperv_features[i]) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of HyperV enlightenment "
@@ -27948,6 +27951,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
case VIR_DOMAIN_HYPERV_FREQUENCIES:
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
case VIR_DOMAIN_HYPERV_TLBFLUSH:
+case VIR_DOMAIN_HYPERV_IPI:
break;
case VIR_DOMAIN_HYPERV_SPINLOCKS:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e30a4b2fe7..9eea75548d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1795,6 +1795,7 @@ typedef enum {
VIR_DOMAIN_HYPERV_FREQUENCIES,
VIR_DOMAIN_HYPERV_REENLIGHTENMENT,
VIR_DOMAIN_HYPERV_TLBFLUSH,
+VIR_DOMAIN_HYPERV_IPI,
VIR_DOMAIN_HYPERV_LAST
} virDomainHyperv;
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 69a0c8db28..33252e927e 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -114,6 +114,8 @@ KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_REENLIGHTENMENT,
0x4003, 0x2000);
KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_TLBFLUSH,
0x4004, 0x0004);
+KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_IPI,
+0x4004, 0x0400);
static virCPUx86Feature x86_kvm_features[] =
{
@@ -137,6 +139,7 @@ static virCPUx86Feature x86_kvm_features[] =
KVM_FEATURE(VIR_CPU_x86_KVM_HV_FREQUENCIES),
KVM_FEATURE(VIR_CPU_x86_KVM_HV_REENLIGHTENMENT),
KVM_FEATURE(VIR_CPU_x86_KVM_HV_TLBFLUSH),
+KVM_FEATURE(VIR_CPU_x86_KVM_HV_IPI),
};
typedef struct _virCPUx86Model virCPUx86Model;
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index e75b3a2d0d..8c51d88e1a 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -65,6 +65,7 @@ struct _virCPUx86CPUID {
# define VIR_CPU_x86_KVM_HV_FREQUENCIES "__kvm_hv_frequencies"
# define VIR_CPU_x86_KVM_HV_REENLIGHTENMENT "__kvm_hv_reenlightenment"
# define VIR_CPU_x86_KVM_HV_TLBFLUSH
[libvirt] [PATCH 0/2] conf: qemu: support new Hyper-V enlightenments in Qemu-3.1
The upcoming Qemu-3.1 release will bring us two new Hyper-V enlightenments: hv_ipi and hv_evmcs. Support these in libvirt. Vitaly Kuznetsov (2): conf: qemu: add support for Hyper-V PV IPIs conf: qemu: add support for Hyper-V Enlightened VMCS docs/formatdomain.html.in | 14 ++ docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c | 10 +- src/conf/domain_conf.h | 2 ++ src/cpu/cpu_x86.c | 6 ++ src/cpu/cpu_x86_data.h | 2 ++ src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_parse_command.c | 2 ++ src/qemu/qemu_process.c | 2 ++ tests/qemuxml2argvdata/hyperv-off.xml | 2 ++ tests/qemuxml2argvdata/hyperv.args | 2 +- tests/qemuxml2argvdata/hyperv.xml | 2 ++ tests/qemuxml2xmloutdata/hyperv-off.xml | 2 ++ tests/qemuxml2xmloutdata/hyperv.xml | 2 ++ 14 files changed, 58 insertions(+), 2 deletions(-) -- 2.17.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] conf: qemu: add support for Hyper-V Enlightened VMCS
Qemu-3.1 supports Hyper-V Enlightened VMCS feature which significantly
speeds up nested Hyper-V on KVM environments.
Signed-off-by: Vitaly Kuznetsov
---
docs/formatdomain.html.in | 7 +++
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c | 6 +-
src/conf/domain_conf.h | 1 +
src/cpu/cpu_x86.c | 3 +++
src/cpu/cpu_x86_data.h | 1 +
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_parse_command.c | 1 +
src/qemu/qemu_process.c | 1 +
tests/qemuxml2argvdata/hyperv-off.xml | 1 +
tests/qemuxml2argvdata/hyperv.args | 2 +-
tests/qemuxml2argvdata/hyperv.xml | 1 +
tests/qemuxml2xmloutdata/hyperv-off.xml | 1 +
tests/qemuxml2xmloutdata/hyperv.xml | 1 +
14 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 24d4b68d7b..ad58f3f1c7 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1982,6 +1982,7 @@
reenlightenment state='on'/
tlbflush state='on'/
ipi state='on'/
+evmcs state='on'/
/hyperv
kvm
hidden state='on'/
@@ -2128,6 +2129,12 @@
on, off
4.10.0 (QEMU 3.1)
+
+ evmcs
+ Enable Enlightened VMCS
+ on, off
+ 4.10.0 (QEMU 3.1)
+
pvspinlock
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 26019b3279..ff57ce1015 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5774,6 +5774,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c8cde90808..b0bb162d46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -173,7 +173,8 @@ VIR_ENUM_IMPL(virDomainHyperv, VIR_DOMAIN_HYPERV_LAST,
"frequencies",
"reenlightenment",
"tlbflush",
- "ipi")
+ "ipi",
+ "evmcs")
VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST,
"hidden")
@@ -19992,6 +19993,7 @@ virDomainDefParseXML(xmlDocPtr xml,
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
case VIR_DOMAIN_HYPERV_TLBFLUSH:
case VIR_DOMAIN_HYPERV_IPI:
+case VIR_DOMAIN_HYPERV_EVMCS:
break;
case VIR_DOMAIN_HYPERV_SPINLOCKS:
@@ -22187,6 +22189,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr
src,
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
case VIR_DOMAIN_HYPERV_TLBFLUSH:
case VIR_DOMAIN_HYPERV_IPI:
+case VIR_DOMAIN_HYPERV_EVMCS:
if (src->hyperv_features[i] != dst->hyperv_features[i]) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of HyperV enlightenment "
@@ -27952,6 +27955,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
case VIR_DOMAIN_HYPERV_TLBFLUSH:
case VIR_DOMAIN_HYPERV_IPI:
+case VIR_DOMAIN_HYPERV_EVMCS:
break;
case VIR_DOMAIN_HYPERV_SPINLOCKS:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9eea75548d..061185e779 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1796,6 +1796,7 @@ typedef enum {
VIR_DOMAIN_HYPERV_REENLIGHTENMENT,
VIR_DOMAIN_HYPERV_TLBFLUSH,
VIR_DOMAIN_HYPERV_IPI,
+VIR_DOMAIN_HYPERV_EVMCS,
VIR_DOMAIN_HYPERV_LAST
} virDomainHyperv;
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 33252e927e..ebfa74fccd 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -116,6 +116,8 @@ KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_TLBFLUSH,
0x4004, 0x0004);
KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_IPI,
0x4004, 0x0400);
+KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_EVMCS,
+0x4004, 0x4000);
static virCPUx86Feature x86_kvm_features[] =
{
@@ -140,6 +142,7 @@ static virCPUx86Feature x86_kvm_features[] =
KVM_FEATURE(VIR_CPU_x86_KVM_HV_REENLIGHTENMENT),
KVM_FEATURE(VIR_CPU_x86_KVM_HV_TLBFLUSH),
KVM_FEATURE(VIR_CPU_x86_KVM_HV_IPI),
+KVM_FEATURE(VIR_CPU_x86_KVM_HV_EVMCS),
};
typedef struct _virCPUx86Model virCPUx86Model;
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 8c51d88e1a..f52bba821f 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -66,6 +66,7 @@ struct _virCPUx86CPUID {
# define VIR_CPU_x86_KVM_HV_REENLIGHTENMENT "__kvm_hv_reenlightenment"
# define VIR_CPU_x86_KVM_HV_TLBFLUSH "__kvm_hv_tlbflush"
# define VIR_CPU_x86_KVM_HV_IPI "__kvm_hv_ipi"
+# define VIR_CPU_x86_KVM_HV_EVMCS "__kvm_hv_evmcs"
#
Re: [libvirt] [PATCH] virSecuritySELinuxTransactionCommit: Return -1 if no transaction is set
On 11/13/2018 05:32 PM, Marc Hartmayer wrote:
> Return -1 and report an error message if no transaction is set and
> virSecuritySELinuxTransactionCommit is called.
>
> The function description of virSecuritySELinuxTransactionCommit says:
>
> "Also it is considered as error if there's no transaction set and this
>function is called."
>
> Signed-off-by: Marc Hartmayer
> Reviewed-by: Boris Fiuczynski
> ---
>
> Please apply this patch after the patch
> "virSecuritySELinuxTransactionCommit: Don't mask error" from Michal.
>
> ---
> src/security/security_selinux.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index c09404f6f833..780d650c69ea 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1094,8 +1094,11 @@
> virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr
> ATTRIBUTE_UNUSED,
> int ret = -1;
>
> list = virThreadLocalGet();
> -if (!list)
> -return 0;
> +if (!list) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("No transaction is set"));
> +return -1;
> +}
>
> if (virThreadLocalSet(, NULL) < 0) {
> virReportSystemError(errno, "%s",
>
He he.
ACKed and pushed both. Thanks for the review.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Fix uninstall when WITH_APPARMOR_PROFILES is defined
On Wed, 2018-11-07 at 17:44 -0700, Jim Fehlig wrote: [...] > @@ -96,6 +96,10 @@ install-apparmor-local: > 'usr.lib.libvirt.virt-aa-helper'" \ > >$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper Pre-existing lack of quoting... I've fixed it with a trivial patch already. > +uninstall-apparmor-local: > + rm -f "$(APPARMOR_LOCAL_DIR)/usr.lib.libvirt.virt-aa-helper" > + rmdir $(APPARMOR_LOCAL_DIR) || : Missing quotes here as well. Once you fix that, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virSecuritySELinuxTransactionCommit: Don't mask error
On Tue, Nov 13, 2018 at 04:55 PM +0100, Michal Privoznik
wrote:
> In 4674fc6afd6 I've implemented transactions for selinux driver.
> Well, now that I am working in this area I've notice a subtle
> bug: @ret is initialized to 0 instead of -1. Facepalm.
>
> Signed-off-by: Michal Privoznik
> ---
>
> I wonder how this could survive this long (~2y) not being noticed.
>
> src/security/security_selinux.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 467d1e6bfe..c09404f6f8 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1091,7 +1091,7 @@
> virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr
> ATTRIBUTE_UNUSED,
> pid_t pid)
> {
> virSecuritySELinuxContextListPtr list;
> -int ret = 0;
> +int ret = -1;
>
> list = virThreadLocalGet();
> if (!list)
> --
> 2.18.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Reviewed-by: Marc Hartmayer
Actually, I had the same fix in my pipeline :)
--
Kind regards / Beste Grüße
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] virSecuritySELinuxTransactionCommit: Return -1 if no transaction is set
Return -1 and report an error message if no transaction is set and
virSecuritySELinuxTransactionCommit is called.
The function description of virSecuritySELinuxTransactionCommit says:
"Also it is considered as error if there's no transaction set and this
function is called."
Signed-off-by: Marc Hartmayer
Reviewed-by: Boris Fiuczynski
---
Please apply this patch after the patch
"virSecuritySELinuxTransactionCommit: Don't mask error" from Michal.
---
src/security/security_selinux.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index c09404f6f833..780d650c69ea 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1094,8 +1094,11 @@
virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
int ret = -1;
list = virThreadLocalGet();
-if (!list)
-return 0;
+if (!list) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("No transaction is set"));
+return -1;
+}
if (virThreadLocalSet(, NULL) < 0) {
virReportSystemError(errno, "%s",
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virSecuritySELinuxTransactionCommit: Don't mask error
In 4674fc6afd6 I've implemented transactions for selinux driver.
Well, now that I am working in this area I've notice a subtle
bug: @ret is initialized to 0 instead of -1. Facepalm.
Signed-off-by: Michal Privoznik
---
I wonder how this could survive this long (~2y) not being noticed.
src/security/security_selinux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 467d1e6bfe..c09404f6f8 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1091,7 +1091,7 @@ virSecuritySELinuxTransactionCommit(virSecurityManagerPtr
mgr ATTRIBUTE_UNUSED,
pid_t pid)
{
virSecuritySELinuxContextListPtr list;
-int ret = 0;
+int ret = -1;
list = virThreadLocalGet();
if (!list)
--
2.18.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 00/14] PCI passthrough support on s390
On Thu, 2018-11-08 at 19:00 +0800, Yi Min Zhao wrote: > Abstract > > The PCI representation in QEMU has been extended for S390 > allowing configuration of zPCI attributes like uid (user-defined > identifier) and fid (PCI function identifier). > The details can be found here: > https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html > > To support the new zPCI feature of the S390 platform, a new element of > PCI address is introduced. It has two optional attributes, @uid and > @fid. For example: > > > > > > > > > > > If they are defined by the user, unique values within the guest domain > must be used. If they are not specified and the architecture requires > them, they are automatically generated with non-conflicting values. > > zPCI address as an extension of the PCI address are stored in a new > structure 'virZPCIDeviceAddress' which is a member of common PCI > Address structure. Additionally, two hashtables are used for assignment > and reservation of zPCI uid/fid. > > In support of extending the PCI address, a new PCI address extension flag is > introduced. This extension flag allows is not only dedicated for the S390 > platform but also other architectures needing certain extensions to PCI > address space. I have now provided R-b for the only patch that was still missing it, and as far as I'm concerned the series is ready to be pushed. Dan, do you have any remaining concerns about the XML syntax, or can I go ahead and push? -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 10/14] conf: Allocate/release 'uid' and 'fid' in PCI address
On Thu, 2018-11-08 at 19:00 +0800, Yi Min Zhao wrote: > This patch adds new functions for reservation, assignment and release > to handle the uid/fid. If the uid/fid is defined in the domain XML, > they will be reserved directly in the collecting phase. If any of them > is not defined, we will find out an available value for them from the > zPCI address hashtable, and reserve them. For the hotplug case there > might not be a zPCI definition. So allocate and reserve uid/fid the > case. Assign if needed and reserve uid/fid for the defined case. > > Signed-off-by: Yi Min Zhao > Reviewed-by: Bjoern Walk > Reviewed-by: Boris Fiuczynski > --- > src/conf/device_conf.c | 16 +++ > src/conf/device_conf.h | 3 + > src/conf/domain_addr.c | 244 + > src/conf/domain_addr.h | 12 ++ > src/libvirt_private.syms | 5 + > src/qemu/qemu_domain_address.c | 59 +++- > 6 files changed, 338 insertions(+), 1 deletion(-) Somehow forgot to give my Reviewed-by: Andrea Bolognani during the previous round of reviews O:-) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] tests: Augment vcgrouptest to add virCgroupGetMemoryStat
On Wed, Nov 07, 2018 at 06:57:40PM -0500, John Ferlan wrote:
> Add a test to fetch the GetMemoryStat output. This only gets
> data for v1 only right now since the v2 data from commit 61ff6021
> is rather useless returning all 0's.
>
> Signed-off-by: John Ferlan
> ---
> tests/vircgrouptest.c | 61 +++
> 1 file changed, 61 insertions(+)
>
> diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
> index 310e1fb6a2..06c4a8ef5c 100644
> --- a/tests/vircgrouptest.c
> +++ b/tests/vircgrouptest.c
> @@ -802,6 +802,64 @@ static int testCgroupGetMemoryUsage(const void *args
> ATTRIBUTE_UNUSED)
> return ret;
> }
>
> +
> +static int
> +testCgroupGetMemoryStat(const void *args ATTRIBUTE_UNUSED)
> +{
> +virCgroupPtr cgroup = NULL;
> +int rv, ret = -1;
Please each variable on separate line. Once you need to change/remove
any of the variable the diff is way better.
> +size_t i;
> +
> +const unsigned long long expected_values[] = {
> +1336619008ULL,
> +67100672ULL,
> +145887232ULL,
> +661872640ULL,
> +627400704UL,
> +3690496ULL
> +};
> +const char* names[] = {
> +"cache",
> +"active_anon",
> +"inactive_anon",
> +"active_file",
> +"inactive_file",
> +"unevictable"
> +};
> +unsigned long long values[ARRAY_CARDINALITY(expected_values)];
> +
> +if ((rv = virCgroupNewPartition("/virtualmachines", true,
> +(1 << VIR_CGROUP_CONTROLLER_MEMORY),
> +)) < 0) {
> +fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
> -rv);
> +goto cleanup;
> +}
> +
> +if ((rv = virCgroupGetMemoryStat(cgroup, [0],
> + [1], [2],
> + [3], [4],
> + [5])) < 0) {
> +fprintf(stderr, "Could not retrieve GetMemoryStat for
> /virtualmachines cgroup: %d\n", -rv);
> +goto cleanup;
> +}
> +
> +for (i = 0; i < ARRAY_CARDINALITY(expected_values); i++) {
> +if (expected_values[i] != (values[i] << 10)) {
This feels wrong and it's just a lucky coincidence that it works with
these values. It's basically the same operation as 'x * 1024'.
I would rather change it into this:
if ((expected_values[i] >> 10) != values[i]) {
because we know that we do the same operation after reading these values
from memory.stat file.
> +fprintf(stderr,
> +"Wrong value (%llu) for %s from virCgroupGetMemoryStat
> (expected %llu)\n",
> +values[i], names[i], expected_values[i]);
This would print wrong values, we need to print shifted values.
Probably the best solution would be to have "expected_values" with the
correct number from the start.
Note: please keep the lines under 80 characters.
Because it's a test I'm OK with both solutions, modifying
"expected_values" in place or to have them correct from the start and
I'll leave it up to you. There is no need to resend it.
Reviewed-by: Pavel Hrdina
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: Fix virCgroupGetMemoryStat
On Wed, Nov 07, 2018 at 06:57:39PM -0500, John Ferlan wrote:
> From: Peter Chubb
>
> Commit 901d2b9c introduced virCgroupGetMemoryStat and replaced
> the LXC virLXCCgroupGetMemStat logic in commit e634c7cd0. However,
> in doing so the replacement wasn't exact as the LXC logic used
> getline() to process the cgroup controller data, while the new
> virCgroupGetMemoryStat used "memory.stat" manual buffer read/
> processing which neglected to forward through @line in order
> to read each line in the output.
>
> To fix that, we should be sure to carry forward the @line value
> for each line read updating it beyond that current @newLine value
> once we've calculated the values that we want.
>
> Signed-off-by: John Ferlan
> ---
> src/util/vircgroupv1.c | 7 ++-
> src/util/vircgroupv2.c | 7 ++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
> index 28a74474ee..678ffda35c 100644
> --- a/src/util/vircgroupv1.c
> +++ b/src/util/vircgroupv1.c
> @@ -1476,7 +1476,7 @@ virCgroupV1GetMemoryStat(virCgroupPtr group,
>
> line = stat;
>
> -while (line) {
> +while (line && *line) {
There is no need to check line, it cannot be NULL.
> char *newLine = strchr(line, '\n');
> char *valueStr = strchr(line, ' ');
> unsigned long long value;
> @@ -1506,6 +1506,11 @@ virCgroupV1GetMemoryStat(virCgroupPtr group,
> inactiveFileVal = value >> 10;
> else if (STREQ(line, "unevictable"))
> unevictableVal = value >> 10;
> +
> +if (newLine)
> +line = newLine + 1;
> +else
> +break;
> }
>
> *cache = cacheVal;
> diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> index 32adb06784..541e8e790e 100644
> --- a/src/util/vircgroupv2.c
> +++ b/src/util/vircgroupv2.c
> @@ -1068,7 +1068,7 @@ virCgroupV2GetMemoryStat(virCgroupPtr group,
>
> line = stat;
>
> -while (line) {
> +while (line && *line) {
Same here.
> char *newLine = strchr(line, '\n');
> char *valueStr = strchr(line, ' ');
> unsigned long long value;
> @@ -1102,6 +1102,11 @@ virCgroupV2GetMemoryStat(virCgroupPtr group,
> inactiveFileVal = value >> 10;
> else if (STREQ(line, "unevictable"))
> unevictableVal = value >> 10;
> +
> +if (newLine)
> +line = newLine + 1;
> +else
> +break;
> }
>
> *cache = cacheVal;
> --
Reviewed-by: Pavel Hrdina
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] travis: Switch from Docker Hub to quay.io
On Tue, Nov 13, 2018 at 01:24:20PM +0100, Andrea Bolognani wrote: > As it's currently impossible for us to create new automated > builds on Docker Hub (see [1]), and quay.io doesn't suffer > from the same problem while still having all the feature we > need, switch to the latter. > > [1] https://github.com/docker/hub-feedback/issues/1676 > > Signed-off-by: Andrea Bolognani > --- > .travis.yml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/.travis.yml b/.travis.yml > index 56d25b2ecd..55ba340a34 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -46,7 +46,7 @@ script: >-e VIR_TEST_DEBUG="$VIR_TEST_DEBUG" >-e MINGW="$MINGW" >-e DISTCHECK_CONFIGURE_FLAGS="$DISTCHECK_CONFIGURE_FLAGS" > - "libvirt/buildenv-$IMAGE" > + "quay.io/libvirt/buildenv-$IMAGE:master" >/bin/sh -xc "$DOCKER_CMD" > > git: Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] travis: Switch from Docker Hub to quay.io
As it's currently impossible for us to create new automated builds on Docker Hub (see [1]), and quay.io doesn't suffer from the same problem while still having all the feature we need, switch to the latter. [1] https://github.com/docker/hub-feedback/issues/1676 Signed-off-by: Andrea Bolognani --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 56d25b2ecd..55ba340a34 100644 --- a/.travis.yml +++ b/.travis.yml @@ -46,7 +46,7 @@ script: -e VIR_TEST_DEBUG="$VIR_TEST_DEBUG" -e MINGW="$MINGW" -e DISTCHECK_CONFIGURE_FLAGS="$DISTCHECK_CONFIGURE_FLAGS" - "libvirt/buildenv-$IMAGE" + "quay.io/libvirt/buildenv-$IMAGE:master" /bin/sh -xc "$DOCKER_CMD" git: -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dockerfiles PATCH 2/2] Drop Fedora 27
The corresponding libvirt-jenkins-ci commit is d3e9f386968a.
Signed-off-by: Andrea Bolognani
---
buildenv-fedora-27.Dockerfile | 80 ---
1 file changed, 80 deletions(-)
delete mode 100644 buildenv-fedora-27.Dockerfile
diff --git a/buildenv-fedora-27.Dockerfile b/buildenv-fedora-27.Dockerfile
deleted file mode 100644
index 73ec23c..000
--- a/buildenv-fedora-27.Dockerfile
+++ /dev/null
@@ -1,80 +0,0 @@
-FROM fedora:27
-ENV PACKAGES audit-libs-devel \
- augeas \
- autoconf \
- automake \
- avahi-devel \
- bash \
- bash-completion \
- ccache \
- chrony \
- cppi \
- cyrus-sasl-devel \
- dbus-devel \
- device-mapper-devel \
- dnsmasq \
- dwarves \
- ebtables \
- fuse-devel \
- gcc \
- gettext \
- gettext-devel \
- git \
- glibc-common \
- glibc-devel \
- glusterfs-api-devel \
- gnutls-devel \
- iproute \
- iproute-tc \
- iscsi-initiator-utils \
- libacl-devel \
- libattr-devel \
- libblkid-devel \
- libcap-ng-devel \
- libcurl-devel \
- libiscsi-devel \
- libnl3-devel \
- libpcap-devel \
- libpciaccess-devel \
- librbd-devel \
- libselinux-devel \
- libssh-devel \
- libssh2-devel \
- libtirpc-devel \
- libtool \
- libudev-devel \
- libwsman-devel \
- libxml2 \
- libxml2-devel \
- libxslt \
- lvm2 \
- make \
- netcf-devel \
- nfs-utils \
- numactl-devel \
- numad \
- parted \
- parted-devel \
- patch \
- perl \
- pkgconfig \
- polkit \
- qemu-img \
- radvd \
- readline-devel \
- rpm-build \
- sanlock-devel \
- screen \
- scrub \
- sheepdog \
- sudo \
- systemtap-sdt-devel \
- vim \
- wireshark-devel \
- xen-devel \
- yajl-devel \
- zfs-fuse
-RUN yum update -y && \
-yum install -y ${PACKAGES} && \
-yum autoremove -y && \
-yum clean all -y
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dockerfiles PATCH 1/2] Add Fedora 29
The corresponding libvirt-jenkins-ci commit is 6777ccab6670.
Signed-off-by: Andrea Bolognani
---
buildenv-fedora-29.Dockerfile | 80 +++
1 file changed, 80 insertions(+)
create mode 100644 buildenv-fedora-29.Dockerfile
diff --git a/buildenv-fedora-29.Dockerfile b/buildenv-fedora-29.Dockerfile
new file mode 100644
index 000..2842691
--- /dev/null
+++ b/buildenv-fedora-29.Dockerfile
@@ -0,0 +1,80 @@
+FROM fedora:29
+ENV PACKAGES audit-libs-devel \
+ augeas \
+ autoconf \
+ automake \
+ avahi-devel \
+ bash \
+ bash-completion \
+ ccache \
+ chrony \
+ cppi \
+ cyrus-sasl-devel \
+ dbus-devel \
+ device-mapper-devel \
+ dnsmasq \
+ dwarves \
+ ebtables \
+ fuse-devel \
+ gcc \
+ gettext \
+ gettext-devel \
+ git \
+ glibc-devel \
+ glusterfs-api-devel \
+ gnutls-devel \
+ iproute \
+ iproute-tc \
+ iscsi-initiator-utils \
+ libacl-devel \
+ libattr-devel \
+ libblkid-devel \
+ libcap-ng-devel \
+ libcurl-devel \
+ libiscsi-devel \
+ libnl3-devel \
+ libpcap-devel \
+ libpciaccess-devel \
+ librbd-devel \
+ libselinux-devel \
+ libssh-devel \
+ libssh2-devel \
+ libtirpc-devel \
+ libtool \
+ libudev-devel \
+ libwsman-devel \
+ libxml2 \
+ libxml2-devel \
+ libxslt \
+ lvm2 \
+ make \
+ netcf-devel \
+ nfs-utils \
+ numactl-devel \
+ numad \
+ parted \
+ parted-devel \
+ patch \
+ perl \
+ pkgconfig \
+ polkit \
+ qemu-img \
+ radvd \
+ readline-devel \
+ rpcgen \
+ rpm-build \
+ sanlock-devel \
+ screen \
+ scrub \
+ sheepdog \
+ sudo \
+ systemtap-sdt-devel \
+ vim \
+ wireshark-devel \
+ xen-devel \
+ yajl-devel \
+ zfs-fuse
+RUN yum update -y && \
+yum install -y ${PACKAGES} && \
+yum autoremove -y && \
+yum clean all -y
--
2.19.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dockerfiles PATCH 0/2] Add Fedora 29, drop Fedora 27
Same as https://www.redhat.com/archives/libvir-list/2018-November/msg00073.html Pushed under the "trivial Dockerfiles update" rule. Andrea Bolognani (2): Add Fedora 29 Drop Fedora 27 ...denv-fedora-27.Dockerfile => buildenv-fedora-29.Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename buildenv-fedora-27.Dockerfile => buildenv-fedora-29.Dockerfile (97%) -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: fix handling of unspecified port in URI
On Tue, Nov 13, 2018 at 11:36:57AM +, Daniel P. Berrangé wrote: > When no server name is provided in the URI, modern versions of libxml2 > will set the port to '-1'. This is a change from behaviour with earlier > versions which set it to 0. > > Libvirt expects the port to be 0 in these cases and as a result we get a > bug when connecting to URIs which lack a server name: > > $ virsh -c test+ssh:///default list > error: failed to connect to the hypervisor > error: Cannot recv data: Bad port '-1': Connection reset by peer > > This libxml2 change was attempting to fix another bug identified by > libvirt where it didn't roundtrip URIs correctly in: > > > https://github.com/GNOME/libxml2/commit/beb7281055dbf0ed4d041022a67c6c5cfd126f25 > > Essentially libxml2 was not expecting apps to look at the URI port > field when the server name is not provided. This was a reasonable > assumption, but none the less libvirt did look at it :-) > > The fix is to ensure we explicitly set port to 0 when server name > is not present, avoiding undefined behaviour for the port field in > libxml2. > > Signed-off-by: Daniel P. Berrangé > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] conf: Move VFIO AP validation from post parse to QEMU validation code
On Tue, Nov 13, 2018 at 10:02:43AM +0100, Boris Fiuczynski wrote:
> On 11/13/18 9:22 AM, Erik Skultety wrote:
> > On Mon, Nov 12, 2018 at 05:39:38PM +0100, Boris Fiuczynski wrote:
> > > On 11/12/18 1:14 PM, Erik Skultety wrote:
> > > > Even though commit 11708641 claims that a domain is allowed have a
> > > > single VFIO AP hostdev only, this is a limitation posed by the platform
> > > > vendor on purely virtual devices. Generally, post parse should only be
> > > I am little confused by the term "purely virtual devices".
> > > If you are talking about the mediated device itself "purely virtual"
> > > sounds
> > > okay but if you also consider what it represents within a guest than that
> > > is
> > > no longer "purely virtual" since a vfio-ap hostdev represents a bunch of
> > > "real crypto hardware" that is passed through to the guest.
> >
> > Yes, I was talking in context of mediated devices themselves, otherwise it
> > would not make sense as you pointed out (not just for AP). So, let's go
> > simple,
> > how about I rewrite the whole commit message in the following manner:
> >
> > "VFIO AP has a limitation on a single device per domain, however, when
> > commit
> > 11708641 added support for vfio-ap, this limitation was performed as part of
> > post parse. Generally, checks like this should be performed within the
> > driver's
> > validation callback to eliminate any possible chance of failing in post
> > parse,
> > which in the worst case could lead to the XML config to vanish."
> >
> > Would you be okay with ^that?
> Yes, it would! :)
>
> >
> > >
> > > > used to populate some default values if missing or at least fail
> > > > gracefully with VIR_DOMAIN_DEF_PARSE_ALLOW_POST_PARSE_FAIL).
> > > >
> > > > This patch more of an attempt to follow the guidelines for adding new
> > > > features rather than a precaution measure, since even if vfio-ap
> > > > supported multiple devices, one would have to downgrade libvirt for a
> > > > machine to vanish from the list or in terms of future device migration
> > > > to slightly older libvirt, there would be most probably a driver
> > > > mismatch
> > > > that would render the migration impossible anyway.
> >
> > I'd then just drop ^this paragraph, doesn't add much info anyway.
> OK
>
> >
> > >
> > > I agree that this is the correct place for the checking. Thanks for
> > > catching
> > > and fixing it. I successfully ran some tests with these changes with
> > > regard
> > > to vfio-ap. Looks good to me so far.
> > >
> > > >
> > > > Signed-off-by: Erik Skultety
> > > > ---
> > > >src/conf/domain_conf.c | 28
> > > >src/qemu/qemu_domain.c | 28 +++-
> > > >2 files changed, 27 insertions(+), 29 deletions(-)
> > > >
> > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > > index 237540bccc..e8e0adc819 100644
> > > > --- a/src/conf/domain_conf.c
> > > > +++ b/src/conf/domain_conf.c
> > > > @@ -4275,31 +4275,6 @@ virDomainDefPostParseGraphics(virDomainDef *def)
> > > >}
> > > > -static int
> > > > -virDomainDefPostParseHostdev(virDomainDefPtr def)
> > > > -{
> > > > -size_t i;
> > > > -bool vfioap_found = false;
> > > > -
> > > > -/* verify settings of hostdevs vfio-ap */
> > > > -for (i = 0; i < def->nhostdevs; i++) {
> > > > -virDomainHostdevDefPtr hostdev = def->hostdevs[i];
> > > > -
> > > > -if (virHostdevIsMdevDevice(hostdev) &&
> > > > -hostdev->source.subsys.u.mdev.model ==
> > > > VIR_MDEV_MODEL_TYPE_VFIO_AP) {
> > > > -if (vfioap_found) {
> > > > -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > > - _("Only one hostdev of model vfio-ap is
> > > > "
> > > > - "supported"));
> > > > -return -1;
> > > > -}
> > > > -vfioap_found = true;
> > > > -}
> > > > -}
> > > > -return 0;
> > > > -}
> > > > -
> > > > -
> > > >/**
> > > > * virDomainDriveAddressIsUsedByDisk:
> > > > * @def: domain definition containing the disks to check
> > > > @@ -5210,9 +5185,6 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
> > > >virDomainDefPostParseGraphics(def);
> > > > -if (virDomainDefPostParseHostdev(def) < 0)
> > > > -return -1;
> > > > -
> > > >if (virDomainDefPostParseCPU(def) < 0)
> > > >return -1;
> > > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > > > index 17d207513d..90253ae867 100644
> > > > --- a/src/qemu/qemu_domain.c
> > > > +++ b/src/qemu/qemu_domain.c
> > > > @@ -4599,6 +4599,32 @@ qemuDomainMdevDefVFIOPCIValidate(const
> > > > virDomainHostdevSubsysMediatedDev *dev,
> > > >}
> > > > +static int
> > > > +qemuDomainMdevDefVFIOAPValidate(const virDomainDef *def)
> > > > +{
> > > > +size_t i;
> > > > +bool vfioap_found = false;
> > > > +
> > > > +/* currently, VFIO-AP is restricted to a
[libvirt] [PATCH] util: fix handling of unspecified port in URI
When no server name is provided in the URI, modern versions of libxml2 will set the port to '-1'. This is a change from behaviour with earlier versions which set it to 0. Libvirt expects the port to be 0 in these cases and as a result we get a bug when connecting to URIs which lack a server name: $ virsh -c test+ssh:///default list error: failed to connect to the hypervisor error: Cannot recv data: Bad port '-1': Connection reset by peer This libxml2 change was attempting to fix another bug identified by libvirt where it didn't roundtrip URIs correctly in: https://github.com/GNOME/libxml2/commit/beb7281055dbf0ed4d041022a67c6c5cfd126f25 Essentially libxml2 was not expecting apps to look at the URI port field when the server name is not provided. This was a reasonable assumption, but none the less libvirt did look at it :-) The fix is to ensure we explicitly set port to 0 when server name is not present, avoiding undefined behaviour for the port field in libxml2. Signed-off-by: Daniel P. Berrangé --- src/util/viruri.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/util/viruri.c b/src/util/viruri.c index d4b793f439..c8811f64c6 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -171,7 +171,16 @@ virURIParse(const char *uri) goto error; if (VIR_STRDUP(ret->server, xmluri->server) < 0) goto error; -ret->port = xmluri->port; +/* xmluri->port value is not defined if server was + * not given. Modern versions libxml2 fill port + * differently to old versions in this case, so + * don't rely on it. eg libxml2 git commit: + * beb7281055dbf0ed4d041022a67c6c5cfd126f25 + */ +if (!ret->server || STREQ(ret->server, "")) +ret->port = 0; +else +ret->port = xmluri->port; if (VIR_STRDUP(ret->path, xmluri->path) < 0) goto error; #ifdef HAVE_XMLURI_QUERY_RAW -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] Add new migration flag VIR_MIGRATE_DRY_RUN
On Mon, Nov 12, 2018 at 11:33:04AM -0700, Jim Fehlig wrote:
> On 11/12/18 4:26 AM, Daniel P. Berrangé wrote:
> > On Fri, Nov 02, 2018 at 04:34:02PM -0600, Jim Fehlig wrote:
> > > A dry run can be used as a best-effort check that a migration command
> > > will succeed. The destination host will be checked to see if it can
> > > accommodate the resources required by the domain. DRY_RUN will fail if
> > > the destination host is not capable of running the domain. Although a
> > > subsequent migration will likely succeed, the success of DRY_RUN does not
> > > ensure a future migration will succeed. Resources on the destination host
> > > could become unavailable between a DRY_RUN and actual migration.
> >
> > I'm not really convinced this is a particularly useful concept,
> > as it is only going to catch a very small number of the reasons
> > why migration can fail. So you still have to expect the real
> > migration invokation to have a strong chance of failing.
>
> I agree it is difficult to reliably check that a migration will succeed.
> TBH, I was expecting opposition due to libvirt already providing info for
> applications to do the check themselves. E.g. as nova has done with
> check_can_live_migrate_{source,destination} APIs.
>
> Do you think libvirt provides enough information for an app to determine if
> a VM can be migrated between two hosts? Or maybe better asked: What info is
> currently missing for an app to reliably check if a VM can be migrated
> between two hosts?
There's probably two classes of problem here
- Things that would prevent the QEMU process being started.
* XML points to host resources that don't exist (block devices,
files, nics, host devs, etc, NUMA/CPU pinning)
* Use of QEMU features that aren't supported by this QEMU version
* Insufficient free resources. Principally lack of RAM,
both normal and huge pages.
These problems are not really anthing todo with live migration
as they impact normal guest startup to exactly the same degree.
Libvirt will already report on the first two problems during
its normal QEMU setup process. During live migration you'll
see these problems reported quite quickly in the prepare phase
before any data is sent.
Insufficient resources is really hard to report on with any
useful accuracy. We can't even predict reliably how much RAM
any given QEMU config will need, let alone measure whether
the host is able to provide that much. If you're lucky QEMU
may simply fail to start due to insufficient RAM/huge pages.
This would abort the live migration early on before much data
is sent.
- Things that interfere with the live migration operation
* Firewall blocks libvirtd <-> libvirtd comms
* Firewall blocks QEMU <-> QEMU comms
* Storage copy is not requested and disks are not
on shared storage
* Network connectivity won't seemlessly switch for
guest NICs
* Bugs in QEMU when loading device state causing
failure
* Bugs in libvirt not correctly configuring QEMU
to ensure stable ABI
* Live migration never converging
Some of these get seen quite quickly such as firewall
issues. Bugs in device state are only seen durnig the
main data transfer. Problems with storage/network
setup are only seen when the guest crashes & burns
after migration is complete & are hard to diagnose
earlier from libvirt's POV. Apps like nova can
diagnose this kind of thing better as they have a
higher level view of the storage/network connectivity
that libvirt can't see.
Live migration convergance is the real hard one
that causes alot of pain for people. Personally I
recommend that people use post-copy by defalt
to guarantee convergance in finite time, with
low impact on guest performance. There was an
interesting presentation at KVM Forum this year
about doing workload prediction for VMs to identify
which time/day has a workload that most friendly
towards convergance.
> > Even launching QEMU isn't good enough - it has to actually process the
> > migration data stream for devices to get a good indication of success,
> > at which point you're basically doing a real migration.
>
> Bummer. I guess that answers my question above: no. It also implies apps
> cannot reliably check if a migration will succeed and should instead put
> effort into handling errors from an actual migration :-).
Yep, we pretty much have to accept that live migration is going to fail
and work to ensure that when it fails, you don't loose the original
VM.
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] Add armv6l Support as guest
From: Stefan Schallenberg
Support for armv6l qemu guests has been added.
Tested with arm1176 CPU on x86.
Signed-off-by Stefan Schallenberg
---
docs/news.xml| 9 +
docs/schemas/basictypes.rng | 1 +
src/qemu/qemu_capabilities.c | 5 ++-
src/qemu/qemu_command.c | 4 +-
src/qemu/qemu_domain.c | 11 --
src/qemu/qemu_domain_address.c | 6 ++-
tests/capabilityschemadata/caps-qemu-kvm.xml | 10 +
tests/testutilsqemu.c| 40 +++-
8 files changed, 75 insertions(+), 11 deletions(-)
diff --git a/docs/news.xml b/docs/news.xml
index 88774c55ae..b01bcbf949 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -25,6 +25,15 @@
+
+
+ Add armv6l Support as guest
+
+
+ Support for armv6l qemu guests has been added.
+ Tested with arm1176 CPU on x86.
+
+
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 14a3670e5c..8a01b854c0 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -380,6 +380,7 @@
aarch64
alpha
armv7l
+ armv6l
cris
i686
ia64
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ca5af3297..4871e89324 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -618,7 +618,7 @@ static const char *virQEMUCapsArchToString(virArch arch)
{
if (arch == VIR_ARCH_I686)
return "i386";
-else if (arch == VIR_ARCH_ARMV7L)
+else if (arch == VIR_ARCH_ARMV6L || arch == VIR_ARCH_ARMV7L)
return "arm";
else if (arch == VIR_ARCH_OR32)
return "or32";
@@ -2179,7 +2179,7 @@ static const char *preferredMachines[] =
{
NULL, /* VIR_ARCH_NONE (not a real arch :) */
"clipper", /* VIR_ARCH_ALPHA */
-NULL, /* VIR_ARCH_ARMV6L (no QEMU impl) */
+"versatilepb", /* VIR_ARCH_ARMV6L */
"integratorcp", /* VIR_ARCH_ARMV7L */
"integratorcp", /* VIR_ARCH_ARMV7B */
@@ -4157,6 +4157,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
/* GIC capabilities, eg. available GIC versions */
if ((qemuCaps->arch == VIR_ARCH_AARCH64 ||
+ qemuCaps->arch == VIR_ARCH_ARMV6L ||
qemuCaps->arch == VIR_ARCH_ARMV7L) &&
virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f59cbf559e..2aa5dab049 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9072,7 +9072,9 @@ static bool
qemuChrIsPlatformDevice(const virDomainDef *def,
virDomainChrDefPtr chr)
{
-if (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) {
+if (def->os.arch == VIR_ARCH_ARMV6L ||
+def->os.arch == VIR_ARCH_ARMV7L ||
+def->os.arch == VIR_ARCH_AARCH64) {
/* pl011 (used on mach-virt) is a platform device */
if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e25afdad6b..2a757683c8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3253,6 +3253,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
addPCIRoot = true;
break;
+case VIR_ARCH_ARMV6L:
case VIR_ARCH_ARMV7L:
case VIR_ARCH_AARCH64:
addDefaultUSB = false;
@@ -3297,7 +3298,6 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
addPCIRoot = true;
break;
-case VIR_ARCH_ARMV6L:
case VIR_ARCH_ARMV7B:
case VIR_ARCH_CRIS:
case VIR_ARCH_ITANIUM:
@@ -5908,7 +5908,8 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
if (ARCH_IS_S390(def->os.arch))
return "virtio";
-if (def->os.arch == VIR_ARCH_ARMV7L ||
+if (def->os.arch == VIR_ARCH_ARMV6L ||
+def->os.arch == VIR_ARCH_ARMV7L ||
def->os.arch == VIR_ARCH_AARCH64) {
if (STREQ(def->os.machine, "versatilepb"))
return "smc91c111";
@@ -9691,7 +9692,8 @@ bool
qemuDomainMachineIsARMVirt(const char *machine,
const virArch arch)
{
-if (arch != VIR_ARCH_ARMV7L &&
+if (arch != VIR_ARCH_ARMV6L &&
+arch != VIR_ARCH_ARMV7L &&
arch != VIR_ARCH_AARCH64)
return false;
@@ -10497,7 +10499,8 @@ qemuDomainSupportsNicdev(virDomainDefPtr def,
virDomainNetDefPtr net)
{
/* non-virtio ARM nics require legacy -net nic */
-if (((def->os.arch == VIR_ARCH_ARMV7L) ||
+if (((def->os.arch == VIR_ARCH_ARMV6L) ||
+(def->os.arch == VIR_ARCH_ARMV7L) ||
(def->os.arch == VIR_ARCH_AARCH64)) &&
net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO &&
net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
diff --git
Re: [libvirt] Parsing of memory.stat in libvirt
> "John" == John Ferlan writes:
On 11/6/18 3:25 AM, peter.ch...@data61.csiro.au wrote:
>>
>> Hi Folks, libvirt currently spams the logs with
>>
>> error : virCgroupGetMemoryStat:2490 : internal error: Cannot parse
>> 'memory.stat' cgroup file
>>
>> whenever someone does ps in a container.
>>
>> This is because the parser for memory/stat is incorrect: the `line'
>> variable is never updated, so each time through the loop, the same
>> start-of-line is compared for the token; and as all spaces are
>> eventually replaced with NUL the error exit is taken instead of
>> ending the loop properly.
>>
>> Here is a strawman patch to fix the problem. Please note, I'm not
>> subscribed to this list; I reported the bug as
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=913023 and was
>> asked to send the patch upstream.
>>
>> src/util/vircgroup.c | 7 ++- 1 file changed, 6 insertions(+), 1
>> deletion(-)
>>
John> H... well I see quite true... nice catch!
John> An any case, the patch below is against an older version of
John> libvirt, we typically work from top of the git tree, but since
John> you're not a regular contributor I will put something together
John> and CC you using top of tree. Since we adhere to usage of
John> Signed-Off-By, I'd need to have you agree to have me add an
John> S-O-B with your name. For now I'll put my name with you as the
John> author.
Sure, I'll add it to the individual patches.
>> --- libvirt.orig/src/util/vircgroup.c +++
>> libvirt/src/util/vircgroup.c @@ -2477,7 +2477,7 @@
>> virCgroupGetMemoryStat(virCgroupPtr grou
>>
>> line = stat;
>>
>> - while (line) { + while (*line) {
John> probably should be line && *line Since if line was for who knows
John> what reason NULL, then life wouldn't be happy.
line can't be NULL at this point. virCgroupGetValueStr() returns
non-zero if it hasn't set stat to something sensible.
An extra check doesn't hurt though.
Peter C
--
Dr Peter Chubb Tel: +61 2 9490 5852 http://ts.data61.csiro.au/
Trustworthy Systems Group Data61, CSIRO (formerly NICTA)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: Fix virCgroupGetMemoryStat
> "John" == John Ferlan writes: John> Signed-off-by: John Ferlan --- Signed-off-by: Peter Chubb -- Dr Peter Chubb Tel: +61 2 9490 5852 http://ts.data61.csiro.au/ Trustworthy Systems Group Data61, CSIRO (formerly NICTA) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Extract MDEV VFIO PCI validation code into a separate helper
On 11/13/18 9:27 AM, Erik Skultety wrote:
On Mon, Nov 12, 2018 at 04:28:09PM +0100, Boris Fiuczynski wrote:
On 11/12/18 1:14 PM, Erik Skultety wrote:
Since we'll need to validate other models apart from VFIO PCI too,
having a helper for each model should keep the code base cleaner.
Signed-off-by: Erik Skultety
---
src/qemu/qemu_domain.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e25afdad6b..17d207513d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4564,11 +4564,11 @@ qemuDomainDeviceDefValidateNetwork(const
virDomainNetDef *net)
static int
-qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
- const virDomainDef *def,
- virQEMUCapsPtr qemuCaps)
+qemuDomainMdevDefVFIOPCIValidate(const virDomainHostdevSubsysMediatedDev *dev,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
{
-if (mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT)
+if (dev->display == VIR_TRISTATE_SWITCH_ABSENT)
return 0;
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY)) {
@@ -4578,7 +4578,7 @@ qemuDomainMdevDefValidate(const
virDomainHostdevSubsysMediatedDev *mdevsrc,
return -1;
}
-if (mdevsrc->model != VIR_MDEV_MODEL_TYPE_VFIO_PCI) {
+if (dev->model != VIR_MDEV_MODEL_TYPE_VFIO_PCI) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_(" attribute 'display' is only supported"
" with model='vfio-pci'"));
@@ -4586,7 +4586,7 @@ qemuDomainMdevDefValidate(const
virDomainHostdevSubsysMediatedDev *mdevsrc,
return -1;
}
-if (mdevsrc->display == VIR_TRISTATE_SWITCH_ON) {
+if (dev->display == VIR_TRISTATE_SWITCH_ON) {
if (def->ngraphics == 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("graphics device is needed for attribute value "
@@ -4599,6 +4599,29 @@ qemuDomainMdevDefValidate(const
virDomainHostdevSubsysMediatedDev *mdevsrc,
}
+static int
+qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+
Syntax-check does not like the above blank line... :-)
Wow, good catch, quite interesting though, because I always build my patches on
a bunch of different distros (a reasonable subset of those in our CI) and none
of them reported this. What's your setup if may ask?
Fedora 28 on s390 running in a VPATH build make check followed by make
syntax-check
+switch ((virMediatedDeviceModelType) mdevsrc->model) {
+case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
+return qemuDomainMdevDefVFIOPCIValidate(mdevsrc, def, qemuCaps);
+case VIR_MDEV_MODEL_TYPE_VFIO_AP:
+break;
+case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
+break;
+case VIR_MDEV_MODEL_TYPE_LAST:
+virReportEnumRangeError(virMediatedDeviceModelType,
+mdevsrc->model);
+return -1;
+}
+
+return 0;
+}
+
+
static int
qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev,
const virDomainDef *def,
Besides Marc question regard the default switch case
Reviewed-by: Boris Fiuczynski
Thanks,
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] conf: Move VFIO AP validation from post parse to QEMU validation code
On 11/13/18 9:22 AM, Erik Skultety wrote:
On Mon, Nov 12, 2018 at 05:39:38PM +0100, Boris Fiuczynski wrote:
On 11/12/18 1:14 PM, Erik Skultety wrote:
Even though commit 11708641 claims that a domain is allowed have a
single VFIO AP hostdev only, this is a limitation posed by the platform
vendor on purely virtual devices. Generally, post parse should only be
I am little confused by the term "purely virtual devices".
If you are talking about the mediated device itself "purely virtual" sounds
okay but if you also consider what it represents within a guest than that is
no longer "purely virtual" since a vfio-ap hostdev represents a bunch of
"real crypto hardware" that is passed through to the guest.
Yes, I was talking in context of mediated devices themselves, otherwise it
would not make sense as you pointed out (not just for AP). So, let's go simple,
how about I rewrite the whole commit message in the following manner:
"VFIO AP has a limitation on a single device per domain, however, when commit
11708641 added support for vfio-ap, this limitation was performed as part of
post parse. Generally, checks like this should be performed within the driver's
validation callback to eliminate any possible chance of failing in post parse,
which in the worst case could lead to the XML config to vanish."
Would you be okay with ^that?
Yes, it would! :)
used to populate some default values if missing or at least fail
gracefully with VIR_DOMAIN_DEF_PARSE_ALLOW_POST_PARSE_FAIL).
This patch more of an attempt to follow the guidelines for adding new
features rather than a precaution measure, since even if vfio-ap
supported multiple devices, one would have to downgrade libvirt for a
machine to vanish from the list or in terms of future device migration
to slightly older libvirt, there would be most probably a driver mismatch
that would render the migration impossible anyway.
I'd then just drop ^this paragraph, doesn't add much info anyway.
OK
I agree that this is the correct place for the checking. Thanks for catching
and fixing it. I successfully ran some tests with these changes with regard
to vfio-ap. Looks good to me so far.
Signed-off-by: Erik Skultety
---
src/conf/domain_conf.c | 28
src/qemu/qemu_domain.c | 28 +++-
2 files changed, 27 insertions(+), 29 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 237540bccc..e8e0adc819 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4275,31 +4275,6 @@ virDomainDefPostParseGraphics(virDomainDef *def)
}
-static int
-virDomainDefPostParseHostdev(virDomainDefPtr def)
-{
-size_t i;
-bool vfioap_found = false;
-
-/* verify settings of hostdevs vfio-ap */
-for (i = 0; i < def->nhostdevs; i++) {
-virDomainHostdevDefPtr hostdev = def->hostdevs[i];
-
-if (virHostdevIsMdevDevice(hostdev) &&
-hostdev->source.subsys.u.mdev.model ==
VIR_MDEV_MODEL_TYPE_VFIO_AP) {
-if (vfioap_found) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Only one hostdev of model vfio-ap is "
- "supported"));
-return -1;
-}
-vfioap_found = true;
-}
-}
-return 0;
-}
-
-
/**
* virDomainDriveAddressIsUsedByDisk:
* @def: domain definition containing the disks to check
@@ -5210,9 +5185,6 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
virDomainDefPostParseGraphics(def);
-if (virDomainDefPostParseHostdev(def) < 0)
-return -1;
-
if (virDomainDefPostParseCPU(def) < 0)
return -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 17d207513d..90253ae867 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4599,6 +4599,32 @@ qemuDomainMdevDefVFIOPCIValidate(const
virDomainHostdevSubsysMediatedDev *dev,
}
+static int
+qemuDomainMdevDefVFIOAPValidate(const virDomainDef *def)
+{
+size_t i;
+bool vfioap_found = false;
+
+/* currently, VFIO-AP is restricted to a single device only */
Well, even so it is just on mdev device it defines the complete set of
crypto devices consisting of adapters, domains and controldomains on the AP
bus of the guest. The ap architecture allows only one such configuration.
So I suggest to remove "currently," and instead of "single device" to write
"single mediated device".
Okay, I should finally read the spec. Anyway, I always tend to look at this
stuff from a larger perspective, in this case, mdev itself - it doesn't have
such a limitation (it may exist within the vendor driver, like NVIDIA and we
obviously don't check that because vfio-pci doesn't have that either). But AP is
different, as I said, I need to look at the spec. I'll adjust according to your
suggestion.
You are right that it is a limit of vfio-ap in qemu and it also enforces
it by throwing an error
Re: [libvirt] [PATCH 1/2] qemu: Extract MDEV VFIO PCI validation code into a separate helper
On Mon, Nov 12, 2018 at 04:28:09PM +0100, Boris Fiuczynski wrote:
> On 11/12/18 1:14 PM, Erik Skultety wrote:
> > Since we'll need to validate other models apart from VFIO PCI too,
> > having a helper for each model should keep the code base cleaner.
> >
> > Signed-off-by: Erik Skultety
> > ---
> > src/qemu/qemu_domain.c | 35 +--
> > 1 file changed, 29 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index e25afdad6b..17d207513d 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -4564,11 +4564,11 @@ qemuDomainDeviceDefValidateNetwork(const
> > virDomainNetDef *net)
> > static int
> > -qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
> > - const virDomainDef *def,
> > - virQEMUCapsPtr qemuCaps)
> > +qemuDomainMdevDefVFIOPCIValidate(const virDomainHostdevSubsysMediatedDev
> > *dev,
> > + const virDomainDef *def,
> > + virQEMUCapsPtr qemuCaps)
> > {
> > -if (mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT)
> > +if (dev->display == VIR_TRISTATE_SWITCH_ABSENT)
> > return 0;
> > if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY)) {
> > @@ -4578,7 +4578,7 @@ qemuDomainMdevDefValidate(const
> > virDomainHostdevSubsysMediatedDev *mdevsrc,
> > return -1;
> > }
> > -if (mdevsrc->model != VIR_MDEV_MODEL_TYPE_VFIO_PCI) {
> > +if (dev->model != VIR_MDEV_MODEL_TYPE_VFIO_PCI) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > _(" attribute 'display' is only supported"
> >" with model='vfio-pci'"));
> > @@ -4586,7 +4586,7 @@ qemuDomainMdevDefValidate(const
> > virDomainHostdevSubsysMediatedDev *mdevsrc,
> > return -1;
> > }
> > -if (mdevsrc->display == VIR_TRISTATE_SWITCH_ON) {
> > +if (dev->display == VIR_TRISTATE_SWITCH_ON) {
> > if (def->ngraphics == 0) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > _("graphics device is needed for attribute
> > value "
> > @@ -4599,6 +4599,29 @@ qemuDomainMdevDefValidate(const
> > virDomainHostdevSubsysMediatedDev *mdevsrc,
> > }
> > +static int
> > +qemuDomainMdevDefValidate(const virDomainHostdevSubsysMediatedDev *mdevsrc,
> > + const virDomainDef *def,
> > + virQEMUCapsPtr qemuCaps)
> > +{
> > +
> Syntax-check does not like the above blank line... :-)
Wow, good catch, quite interesting though, because I always build my patches on
a bunch of different distros (a reasonable subset of those in our CI) and none
of them reported this. What's your setup if may ask?
>
> > +switch ((virMediatedDeviceModelType) mdevsrc->model) {
> > +case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
> > +return qemuDomainMdevDefVFIOPCIValidate(mdevsrc, def, qemuCaps);
> > +case VIR_MDEV_MODEL_TYPE_VFIO_AP:
> > +break;
> > +case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
> > +break;
> > +case VIR_MDEV_MODEL_TYPE_LAST:
> > +virReportEnumRangeError(virMediatedDeviceModelType,
> > +mdevsrc->model);
> > +return -1;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +
> > static int
> > qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev,
> > const virDomainDef *def,
> >
> Besides Marc question regard the default switch case
> Reviewed-by: Boris Fiuczynski
Thanks,
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] conf: Move VFIO AP validation from post parse to QEMU validation code
On Mon, Nov 12, 2018 at 05:39:38PM +0100, Boris Fiuczynski wrote:
> On 11/12/18 1:14 PM, Erik Skultety wrote:
> > Even though commit 11708641 claims that a domain is allowed have a
> > single VFIO AP hostdev only, this is a limitation posed by the platform
> > vendor on purely virtual devices. Generally, post parse should only be
> I am little confused by the term "purely virtual devices".
> If you are talking about the mediated device itself "purely virtual" sounds
> okay but if you also consider what it represents within a guest than that is
> no longer "purely virtual" since a vfio-ap hostdev represents a bunch of
> "real crypto hardware" that is passed through to the guest.
Yes, I was talking in context of mediated devices themselves, otherwise it
would not make sense as you pointed out (not just for AP). So, let's go simple,
how about I rewrite the whole commit message in the following manner:
"VFIO AP has a limitation on a single device per domain, however, when commit
11708641 added support for vfio-ap, this limitation was performed as part of
post parse. Generally, checks like this should be performed within the driver's
validation callback to eliminate any possible chance of failing in post parse,
which in the worst case could lead to the XML config to vanish."
Would you be okay with ^that?
>
> > used to populate some default values if missing or at least fail
> > gracefully with VIR_DOMAIN_DEF_PARSE_ALLOW_POST_PARSE_FAIL).
> >
> > This patch more of an attempt to follow the guidelines for adding new
> > features rather than a precaution measure, since even if vfio-ap
> > supported multiple devices, one would have to downgrade libvirt for a
> > machine to vanish from the list or in terms of future device migration
> > to slightly older libvirt, there would be most probably a driver mismatch
> > that would render the migration impossible anyway.
I'd then just drop ^this paragraph, doesn't add much info anyway.
>
> I agree that this is the correct place for the checking. Thanks for catching
> and fixing it. I successfully ran some tests with these changes with regard
> to vfio-ap. Looks good to me so far.
>
> >
> > Signed-off-by: Erik Skultety
> > ---
> > src/conf/domain_conf.c | 28
> > src/qemu/qemu_domain.c | 28 +++-
> > 2 files changed, 27 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 237540bccc..e8e0adc819 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -4275,31 +4275,6 @@ virDomainDefPostParseGraphics(virDomainDef *def)
> > }
> > -static int
> > -virDomainDefPostParseHostdev(virDomainDefPtr def)
> > -{
> > -size_t i;
> > -bool vfioap_found = false;
> > -
> > -/* verify settings of hostdevs vfio-ap */
> > -for (i = 0; i < def->nhostdevs; i++) {
> > -virDomainHostdevDefPtr hostdev = def->hostdevs[i];
> > -
> > -if (virHostdevIsMdevDevice(hostdev) &&
> > -hostdev->source.subsys.u.mdev.model ==
> > VIR_MDEV_MODEL_TYPE_VFIO_AP) {
> > -if (vfioap_found) {
> > -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > - _("Only one hostdev of model vfio-ap is "
> > - "supported"));
> > -return -1;
> > -}
> > -vfioap_found = true;
> > -}
> > -}
> > -return 0;
> > -}
> > -
> > -
> > /**
> >* virDomainDriveAddressIsUsedByDisk:
> >* @def: domain definition containing the disks to check
> > @@ -5210,9 +5185,6 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
> > virDomainDefPostParseGraphics(def);
> > -if (virDomainDefPostParseHostdev(def) < 0)
> > -return -1;
> > -
> > if (virDomainDefPostParseCPU(def) < 0)
> > return -1;
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 17d207513d..90253ae867 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -4599,6 +4599,32 @@ qemuDomainMdevDefVFIOPCIValidate(const
> > virDomainHostdevSubsysMediatedDev *dev,
> > }
> > +static int
> > +qemuDomainMdevDefVFIOAPValidate(const virDomainDef *def)
> > +{
> > +size_t i;
> > +bool vfioap_found = false;
> > +
> > +/* currently, VFIO-AP is restricted to a single device only */
> Well, even so it is just on mdev device it defines the complete set of
> crypto devices consisting of adapters, domains and controldomains on the AP
> bus of the guest. The ap architecture allows only one such configuration.
> So I suggest to remove "currently," and instead of "single device" to write
> "single mediated device".
Okay, I should finally read the spec. Anyway, I always tend to look at this
stuff from a larger perspective, in this case, mdev itself - it doesn't have
such a limitation (it may exist within the vendor driver, like NVIDIA and we
obviously don't check that because vfio-pci
