Re: [PATCH v2 00/14] news update since v6.9 to v7.0
ping On Thu, Apr 22, 2021 at 11:48 AM Han Han wrote: > Diff from v1: > - Drop the news "Introduce VIR_DOMAIN_CHECKPOINT_REDEFINE_VALIDATE flag" > - Move the news of virt-aa-helper to bug fix part > - Update some descriptions of news > > v1: > https://listman.redhat.com/archives/libvir-list/2021-April/msg00456.html > > Thanks for the advice from Peter Krempa and Erik Skultety. > > Han Han (14): > news: make SEV attrs 'cbitpos' & 'reducedPhysBits' optional > news: support device stats collection for SR-IOV VF hostdev > news: virt-aa-helper: allow guest to create hard links for mounted > 9pfs paths > news: cpu_map: Add EPYC-Rome cpu model > news: cpu: Support for XML validation in cpu comparison > logging: allow max_len=0 to disable log rollover > news: qemu: Set noqueue qdisc for TAP devices > news: qemu: Introduce virtio free page reporting feature > news: qemu: virtiofs can be used without NUMA nodes > news: qemu: Add 'fmode' and 'dmode' options for 9pfs > news: Introduce "migrate_tls_force" to qemu.conf > qemu: support kvm-poll-control performance hint > news: cpu_map: Add Snowridge cpu model > news: qemu: Add support for NFS disk protocol > > NEWS.rst | 74 > 1 file changed, 74 insertions(+) > > -- > 2.31.1 > >
Re: [PATCH v1] libxl: fix refcounting in libxlDomainChangeEjectableMedia
On 5/18/21 1:26 PM, Olaf Hering wrote:
The initial variant of libxlDomainChangeEjectableMedia could just leave
the function earlier. With refcounting this does not work anymore.
Fixes commit a5bf06ba34dbb226ac1b2fb63f5026c5d493bc65
Signed-off-by: Olaf Hering
Reviewed-by: Jim Fehlig
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 99a170ff2a..d54cd41785 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2997,7 +2997,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm,
virDomainDiskDef *disk)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Removable media not supported for %s device"),
virDomainDiskDeviceTypeToString(disk->device));
-return -1;
+goto cleanup;
}
if (libxlMakeDisk(disk, _disk) < 0)
Re: [PATCH v2] libxl: set vcpu affinity during domain creation
Back from some time off...
On 5/5/21 8:06 AM, Olaf Hering wrote:
Since Xen 4.5 libxl allows to set affinities during domain creation.
This enables Xen to allocate the domain memory on NUMA systems close to
the specified pcpus.
Libvirt can now handle in domU.xml correctly.
Without this change, Xen will create the domU and assign NUMA memory and
vcpu affinities on its own. Later libvirt will adjust the affinity,
which may move the vcpus away from the assigned NUMA node.
Signed-off-by: Olaf Hering
I see Daniel reviewed this patch but it had not been pushed. I've added the r-b
and pushed it.
Thanks!
Jim
---
src/libxl/libxl_conf.c | 53
src/libxl/libxl_domain.c | 46 --
src/libxl/libxl_domain.h | 4 ---
3 files changed, 53 insertions(+), 50 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 6392d7795d..2a99626f92 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -283,6 +283,56 @@ libxlMakeChrdevStr(virDomainChrDef *def, char **buf)
return 0;
}
+static int
+libxlSetVcpuAffinities(virDomainDef *def,
+ libxl_ctx *ctx,
+ libxl_domain_build_info *b_info)
+{
+libxl_bitmap *vcpu_affinity_array;
+unsigned int vcpuid;
+unsigned int vcpu_idx = 0;
+virDomainVcpuDef *vcpu;
+bool has_vcpu_pin = false;
+
+/* Get highest vcpuid with cpumask */
+for (vcpuid = 0; vcpuid < b_info->max_vcpus; vcpuid++) {
+vcpu = virDomainDefGetVcpu(def, vcpuid);
+if (!vcpu)
+continue;
+if (!vcpu->cpumask)
+continue;
+vcpu_idx = vcpuid;
+has_vcpu_pin = true;
+}
+/* Nothing to do */
+if (!has_vcpu_pin)
+return 0;
+
+/* Adjust index */
+vcpu_idx++;
+
+b_info->num_vcpu_hard_affinity = vcpu_idx;
+/* Will be released by libxl_domain_config_dispose */
+b_info->vcpu_hard_affinity = g_new0(libxl_bitmap, vcpu_idx);
+vcpu_affinity_array = b_info->vcpu_hard_affinity;
+
+for (vcpuid = 0; vcpuid < vcpu_idx; vcpuid++) {
+libxl_bitmap *map = _affinity_array[vcpuid];
+libxl_bitmap_init(map);
+/* libxl owns the bitmap */
+if (libxl_cpu_bitmap_alloc(ctx, map, 0))
+return -1;
+vcpu = virDomainDefGetVcpu(def, vcpuid);
+/* Apply the given mask, or allow unhandled vcpus to run anywhere */
+if (vcpu && vcpu->cpumask)
+virBitmapToDataBuf(vcpu->cpumask, map->map, map->size);
+else
+libxl_bitmap_set_any(map);
+}
+libxl_defbool_set(_info->numa_placement, false);
+return 0;
+}
+
static int
libxlMakeDomBuildInfo(virDomainDef *def,
libxlDriverConfig *cfg,
@@ -320,6 +370,9 @@ libxlMakeDomBuildInfo(virDomainDef *def,
for (i = 0; i < virDomainDefGetVcpus(def); i++)
libxl_bitmap_set((_info->avail_vcpus), i);
+if (libxlSetVcpuAffinities(def, ctx, b_info))
+return -1;
+
switch ((virDomainClockOffsetType) clock.offset) {
case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
if (clock.data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index d78765ad84..625e04a9b0 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -967,49 +967,6 @@ libxlDomainAutoCoreDump(libxlDriverPrivate *driver,
return 0;
}
-int
-libxlDomainSetVcpuAffinities(libxlDriverPrivate *driver, virDomainObj *vm)
-{
-g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
-virDomainVcpuDef *vcpu;
-libxl_bitmap map;
-virBitmap *cpumask = NULL;
-size_t i;
-int ret = -1;
-
-libxl_bitmap_init();
-
-for (i = 0; i < virDomainDefGetVcpus(vm->def); ++i) {
-vcpu = virDomainDefGetVcpu(vm->def, i);
-
-if (!vcpu->online)
-continue;
-
-if (!(cpumask = vcpu->cpumask))
-cpumask = vm->def->cpumask;
-
-if (!cpumask)
-continue;
-
-if (virBitmapToData(cpumask, , (int *)) < 0)
-goto cleanup;
-
-if (libxl_set_vcpuaffinity(cfg->ctx, vm->def->id, i, , NULL) != 0)
{
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to pin vcpu '%zu' with libxenlight"), i);
-goto cleanup;
-}
-
-libxl_bitmap_dispose(); /* Also returns to freshly-init'd state */
-}
-
-ret = 0;
-
- cleanup:
-libxl_bitmap_dispose();
-return ret;
-}
-
static int
libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config)
{
@@ -1460,9 +1417,6 @@ libxlDomainStart(libxlDriverPrivate *driver,
goto destroy_dom;
}
-if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
-goto destroy_dom;
-
if (!start_paused) {
libxlDomainUnpauseWrapper(cfg->ctx, domid);
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
[PATCH v1] libxl: fix refcounting in libxlDomainChangeEjectableMedia
The initial variant of libxlDomainChangeEjectableMedia could just leave
the function earlier. With refcounting this does not work anymore.
Fixes commit a5bf06ba34dbb226ac1b2fb63f5026c5d493bc65
Signed-off-by: Olaf Hering
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 99a170ff2a..d54cd41785 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2997,7 +2997,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm,
virDomainDiskDef *disk)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Removable media not supported for %s device"),
virDomainDiskDeviceTypeToString(disk->device));
-return -1;
+goto cleanup;
}
if (libxlMakeDisk(disk, _disk) < 0)
Re: RFC: Qemu backup interface plans
Hi, Your proposal sounds good to me in general. Some small independent building blocks that seems to make sense to me. On 17.05.21 14:07, Vladimir Sementsov-Ogievskiy wrote: [...] What we lack in this scheme: 1. handling dirty bitmap in backup-top filter: backup-top does copy-before-write operation on any guest write, when actually we are interested only in "dirty" regions for incremental backup Probable solution would allowing specifying bitmap for sync=none mode of backup, but I think what I propose below is better. 2. [actually it's a tricky part of 1]: possibility to not do copy-before-write operations for regions that was already copied to final backup. With normal Qemu backup job, this is achieved by the fact that block-copy state with its internal bitmap is shared between backup job and copy-before-write filter. 3. Not a real problem but fact: backup block-job does nothing in the scheme, the whole job is done by filter. So, it would be interesting to have a possibility to simply insert/remove the filter, and avoid block-job creation and managing at all for external backup. (and I'd like to send another RFC on how to insert/remove filters, let's not discuss it here). Next. Think about internal backup. It has one drawback too: 4. If target is remote with slow connection, copy-before-write operations will slow down guest writes appreciably. It may be solved with help of image fleecing: we create temporary qcow2 image, setup fleecing scheme, and instead of exporting temp image through NBD we start a second backup with source = temporary image and target would be real backup target (NBD for example). How would a second backup work here? Wouldn’t one want a mirror job to copy the data off to the real target? (Because I see backup as something intrinsically synchronous, whereas mirror by default is rather lazy.) [Note from future me where I read more below: I see you acknowledge that you’ll need to modify backup to do what you need here, i.e. not do any CBW operations. So it’s effectively the same as a mirror that ignores new dirty areas. Which could work without changing mirror if block-copy were to set BDRV_REQ_WRITE_UNCHANGED for the fleecing case, and bdrv_co_write_req_finish() would skip bdrv_set_dirty() for such writes.] I mean, still has the problem that the mirror job can’t tell the CBW filter which areas are already copied off and so don’t need to be preserved anymore, but... Still, with such solution there are same [1,2] problems, 3 becomes worse: Not sure how 3 can become worse when you said above it isn’t a real problem (to which I agree). 5. We'll have two jobs and two automatically inserted filters, when actually one filter and one job are enough (as first job is needed only to insert a filter, second job doesn't need a filter at all). Note also, that this (starting two backup jobs to make push backup with fleecing) doesn't work now, op-blockers will be against. It's simple to fix (and in Virtuozzo we live with downstream-only patch, which allows push backup with fleecing, based on starting two backup jobs).. But I never send a patch, as I have better plan, which will solve all listed problems. So, what I propose: 1. We make backup-top filter public, so that it could be inserted/removed where user wants through QMP (how to properly insert/remove filter I'll post another RFC, as backup-top is not the only filter that can be usefully inserted somewhere). For this first step I've sent a series today: subject: [PATCH 00/21] block: publish backup-top filter id: <20210517064428.16223-1-vsement...@virtuozzo.com> patchew: https://patchew.org/QEMU/20210517064428.16223-1-vsement...@virtuozzo.com/ (note, that one of things in this series is rename s/backup-top/copy-before-write/, still, I call it backup-top in this letter) This solves [3]. [4, 5] are solved partly: we still have one extra filter, created by backup block jobs, and also I didn't test does this work, probably some op-blockers or permissions should be tuned. So, let it be step 2: 2. Test, that we can start backup job with source = (target of backup-top filter), so that we have "push backup with fleecing". Make an option for backup to start without a filter, when we don't need copy-before-write operations, to not create extra superfluous filter. OK, so the backup job is not really a backup job, but just anything that copies data. 3. Support bitmap in backup-top filter, to solve [1] 3.1 and make it possible to modify the bitmap externally, so that consumer of fleecing can say to backup-top filter: I've already copied these blocks, don't bother with copying them to temp image". This is to solve [2]. Still, how consumer of fleecing will reset shared bitmap after copying blocks? I have the following idea: we make a "discard-bitmap-filter" filter driver, that own some bitmap and on discard request unset corresponding bits.
Re: Qemu block filter insertion/removal API
On 17.05.21 14:44, Vladimir Sementsov-Ogievskiy wrote: Hi all! I'd like to be sure that we know where we are going to. In blockdev-era where qemu user is aware about block nodes, all nodes have good names and controlled by user we can efficiently use block filters. We already have some useful filters: copy-on-read, throttling, compress. In my parallel series I make backup-top filter public and useful without backup block jobs. But now filters could be inserted only together with opening their child. We can specify filters in qemu cmdline, or filter can take place in the block node chain created by blockdev-add. Still, it would be good to insert/remove filters on demand. Currently we are going to use x-blockdev-reopen for this. Still it can't be used to insert a filter above root node (as x-blockdev-reopen can change only block node options and their children). In my series "[PATCH 00/21] block: publish backup-top filter" I propose (as Kevin suggested) to modify qom-set, so that it can set drive option of running device. That's not difficult, but it means that we have different scenario of inserting/removing filters: 1. filter above root node X: inserting: - do blockdev-add to add a filter (and specify X as its child) - do qom-set to set new filter as a rood node instead of X removing - do qom-set to make X a root node again - do blockdev-del to drop a filter 2. filter between two block nodes P and X. (For example, X is a backing child of P) inserting - do blockdev-add to add a filter (and specify X as its child) - do blockdev-reopen to set P.backing = filter remvoing - do blockdev-reopen to set P.backing = X - do blockdev-del to drop a filter And, probably we'll want transaction support for all these things. Is it OK? Or do we need some kind of additional blockdev-replace command, that can replace one node by another, so in both cases we will do inserting: - blockdev-add filter - blockdev-replace (make all parents of X to point to the new filter instead (except for the filter itself of course) removing - blockdev-replace (make all parante of filter to be parents of X instead) - blockdev-del filter It's simple to implement, and it seems for me that it is simpler to use. Any thoughts? I’m afraid as a non-user of the blockdev interface, I can’t give a valuable opinion that would have some actual weight. Doesn’t stop me from giving my personal and potentially invaluable opinion, though, obviously: I think we expect all users to know the block graph, so they should be able to distinguish between cases 1 and 2. However, I can imagine having to distinguish still is kind of a pain, especially if it were trivial for qemu to let the user not having to worry about it at all. Also, if you want a filter unconditionally above some node, all the qom-set and blockdev-reopen operations for all of the original node’s parents would need to happen atomically. As you say, those operations should perhaps be transactionable anyway, but... Implementing blockdev-replace would provide this for much less cost now, I suppose? I guess it can be argued that the downside is that having blockdev-replace means less pressure to make qom-set for drive and blockdev-reopen transactionable. But well. I don’t really have anything against a blockdev-replace, but again, I don’t know whether my opinion on this topic really has weight. Max
Re: [libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part XI
Reviewed-by: Laine Stump I'll push when the CI on my review branch is finished. On 5/18/21 11:04 AM, Tim Wiederhake wrote: For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainHostdevDef: Change type of startupPolicy to virDomainStartupPolicy virDomainHostdevSubsysUSBDefParseXML: Use virXMLProp* virDomainDeviceUSBMasterParseXML: Use virXMLProp* virDomainDiskDef: Change type of geometry.trans to virDomainDiskGeometryTrans virDomainDiskDefGeometryParse: Use virXMLProp* virDomainChrSourceReconnectDefParseXML: Use virXMLProp* virDomainChrDefParseTargetXML: Use virXMLProp* virDomainAudioCoreAudioParse: Use virXMLProp* virDomainAudioOSSParse: Use virXMLProp* virNodeDevCapPCIDevIommuGroupParseXML: Use virXMLProp* src/conf/domain_conf.c | 168 +--- src/conf/domain_conf.h | 23 +++-- src/conf/node_device_conf.c | 15 +--- 3 files changed, 52 insertions(+), 154 deletions(-)
Re: [libvirt PATCH 02/10] virDomainHostdevSubsysUSBDefParseXML: Use virXMLProp*
On 5/18/21 11:04 AM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 734fa584a4..661fa53206 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6694,28 +6694,23 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
virDomainHostdevDef *def)
{
virDomainHostdevSubsysUSB *usbsrc = >source.subsys.u.usb;
-g_autofree char *startupPolicy = NULL;
-g_autofree char *autoAddress = NULL;
xmlNodePtr vendorNode;
xmlNodePtr productNode;
xmlNodePtr addressNode;
+virTristateBool autoAddress;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
ctxt->node = node;
-if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
-int value = virDomainStartupPolicyTypeFromString(startupPolicy);
-if (value <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown startup policy '%s'"),
- startupPolicy);
-return -1;
-}
-def->startupPolicy = value;
-}
+if (virXMLPropEnum(node, "startupPolicy",
+ virDomainStartupPolicyTypeFromString,
+ VIR_XML_PROP_NONZERO, >startupPolicy) < 0)
+return -1;
-if ((autoAddress = virXMLPropString(node, "autoAddress")))
-ignore_value(virStringParseYesNo(autoAddress, >autoAddress));
+if (virXMLPropTristateBool(node, "autoAddress", VIR_XML_PROP_NONE,
+ ) < 0)
+return -1;
+usbsrc->autoAddress = autoAddress == VIR_TRISTATE_BOOL_YES;
(my poor eyesight kept seeing that "==" as "=" and I was confused for a
second! :-)
Too bad we have to do this rather than having a virTristateBool directly
in the object. But history doesn't permit it (without some *other* extra
code to set it to ..._NO in the case when it isn't specified).
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized */
Re: [PATCH] conf: Report alias name of the detached device in error
On 5/18/21 5:44 AM, Kristina Hanicova wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942367
Signed-off-by: Kristina Hanicova
---
src/conf/domain_conf.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7044701fac..e21b9fb946 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15781,38 +15781,45 @@ virDomainNetFindIdx(virDomainDef *def,
virDomainNetDef *net)
if (matchidx < 0) {
if (MACAddrSpecified && PCIAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device matching MAC address %s found on "
+ _("no device matching MAC address %s and alias %s found
on "
VIR_PCI_DEVICE_ADDRESS_FMT),
virMacAddrFormat(>mac, mac),
+ NULLSTR(net->info.alias),
net->info.addr.pci.domain,
net->info.addr.pci.bus,
net->info.addr.pci.slot,
net->info.addr.pci.function);
} else if (MACAddrSpecified && CCWAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device matching MAC address %s found on "
+ _("no device matching MAC address %s and alias %s found
on "
These messages will look strange in the (most common) case where alias
isn't specified, e.g.:
no device matching MAC address DE:AD:BE:EF:01:10
and alias found on [some CCW address]
On the other hand, the idea of even further exploding this bunch of
conditionals to include all combinations is just horrible to think about!
What about instead reworking this to use a single virReportError() that
references a few pointers setup beforehand and then substituting (a
properly i8n'ized!) "(unspecified)" for each item that hasn't been
specified, e.g.:
g_autofree *addr = g_strdup(_("(unspecified)"));
const char *mac = _("(unspecified)");
const char *alias = _("(unspecified)");
if (MACAddrSpecified)
mac = virMacAddrFormat(>mac, mac);
if (net->info.alias)
alias = net->info.alias
if (CCWAddrSpecified)
addr = virCCWAddressAsString(blah);
else if (PCIAddrSpecified)
addr = virPCIDeviceAddressAsString(blah);
virReportError(blah...
_("no device found at address '%s' matching MAC address
'%s' and alias '%s'"),
addr, mac, alias);
or something like that. It's still not ideal, but avoids the conditional
explosion and I think is less confusing than having "alias" followed by
nothing.
VIR_CCW_DEVICE_ADDRESS_FMT),
virMacAddrFormat(>mac, mac),
+ NULLSTR(net->info.alias),
net->info.addr.ccw.cssid,
net->info.addr.ccw.ssid,
net->info.addr.ccw.devno);
} else if (PCIAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device found on " VIR_PCI_DEVICE_ADDRESS_FMT),
+ _("no device matching alias %s found on "
+ VIR_PCI_DEVICE_ADDRESS_FMT),
+ NULLSTR(net->info.alias),
net->info.addr.pci.domain,
net->info.addr.pci.bus,
net->info.addr.pci.slot,
net->info.addr.pci.function);
} else if (CCWAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device found on " VIR_CCW_DEVICE_ADDRESS_FMT),
+ _("no device matching alias %s found on "
+ VIR_CCW_DEVICE_ADDRESS_FMT),
+ NULLSTR(net->info.alias),
net->info.addr.ccw.cssid,
net->info.addr.ccw.ssid,
net->info.addr.ccw.devno);
} else if (MACAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device matching MAC address %s found"),
- virMacAddrFormat(>mac, mac));
+ _("no device matching MAC address %s and alias %s
found"),
+ virMacAddrFormat(>mac, mac),
+ NULLSTR(net->info.alias));
} else {
virReportError(VIR_ERR_DEVICE_MISSING, "%s",
_("no matching device found"));
Re: [PATCH 0/2] Finish min QEMU version bump
On Tue, May 18, 2021 at 15:21:22 +0200, Michal Privoznik wrote: > *** BLURB HERE *** > > Michal Prívozník (2): > qemu_capabilities: Update QEMU_MIN_* macros > domaincapsdata: Drop expected outputs for old QEMUs Oops, Reviewed-by: Peter Krempa
[libvirt PATCH 10/10] virNodeDevCapPCIDevIommuGroupParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `number`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/node_device_conf.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 861f43f6c4..332b12f997 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1557,25 +1557,14 @@
virNodeDevCapPCIDevIommuGroupParseXML(xmlXPathContextPtr ctxt,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
g_autofree xmlNodePtr *addrNodes = NULL;
-g_autofree char *numberStr = NULL;
int nAddrNodes;
size_t i;
ctxt->node = iommuGroupNode;
-numberStr = virXMLPropString(iommuGroupNode, "number");
-if (!numberStr) {
-virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing iommuGroup number attribute"));
+if (virXMLPropUInt(iommuGroupNode, "number", 10, VIR_XML_PROP_REQUIRED,
+ _dev->iommuGroupNumber) < 0)
return -1;
-}
-if (virStrToLong_ui(numberStr, NULL, 10,
-_dev->iommuGroupNumber) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid iommuGroup number attribute '%s'"),
- numberStr);
-return -1;
-}
if ((nAddrNodes = virXPathNodeSet("./address", ctxt, )) < 0)
return -1;
--
2.26.3
[libvirt PATCH 06/10] virDomainChrSourceReconnectDefParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `timeout`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 29 +
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bfcc56ca9e..a5514660cc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10093,39 +10093,20 @@
virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDef *def,
xmlNodePtr node,
xmlXPathContextPtr ctxt)
{
-int tmpVal;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
xmlNodePtr cur;
-g_autofree char *tmp = NULL;
ctxt->node = node;
if ((cur = virXPathNode("./reconnect", ctxt))) {
-if ((tmp = virXMLPropString(cur, "enabled"))) {
-if ((tmpVal = virTristateBoolTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid reconnect enabled value: '%s'"),
- tmp);
-return -1;
-}
-def->enabled = tmpVal;
-VIR_FREE(tmp);
-}
+if (virXMLPropTristateBool(cur, "enabled", VIR_XML_PROP_NONE,
+ >enabled) < 0)
+return -1;
if (def->enabled == VIR_TRISTATE_BOOL_YES) {
-if ((tmp = virXMLPropString(cur, "timeout"))) {
-if (virStrToLong_ui(tmp, NULL, 10, >timeout) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid reconnect timeout value: '%s'"),
- tmp);
-return -1;
-}
-} else {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing timeout for chardev with "
- "reconnect enabled"));
+if (virXMLPropUInt(cur, "timeout", 10, VIR_XML_PROP_REQUIRED,
+ >timeout) < 0)
return -1;
-}
}
}
--
2.26.3
[libvirt PATCH 09/10] virDomainAudioOSSParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 19 ---
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a46e64c64a..b3ed3a9c5a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13074,26 +13074,15 @@ static int
virDomainAudioOSSParse(virDomainAudioIOOSS *def,
xmlNodePtr node)
{
-g_autofree char *tryPoll = virXMLPropString(node, "tryPoll");
-g_autofree char *bufferCount = virXMLPropString(node, "bufferCount");
-
def->dev = virXMLPropString(node, "dev");
-if (tryPoll &&
-((def->tryPoll =
- virTristateBoolTypeFromString(tryPoll)) <= 0)) {
-virReportError(VIR_ERR_XML_ERROR,
- _("unknown 'tryPoll' value '%s'"), tryPoll);
+if (virXMLPropTristateBool(node, "tryPoll", VIR_XML_PROP_NONE,
+ >tryPoll) < 0)
return -1;
-}
-if (bufferCount &&
-virStrToLong_ui(bufferCount, NULL, 10,
->bufferCount) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse 'bufferCount' value '%s'"),
bufferCount);
+if (virXMLPropUInt(node, "bufferCount", 10, VIR_XML_PROP_NONE,
+ >bufferCount) < 0)
return -1;
-}
return 0;
}
--
2.26.3
[libvirt PATCH 07/10] virDomainChrDefParseTargetXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `port`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 29 +
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a5514660cc..57a54f12ef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10977,7 +10977,6 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
g_autofree char *targetModel = NULL;
g_autofree char *addrStr = NULL;
g_autofree char *portStr = NULL;
-g_autofree char *stateStr = NULL;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
ctxt->node = cur;
@@ -11007,7 +11006,6 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
switch (def->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
addrStr = virXMLPropString(cur, "address");
-portStr = virXMLPropString(cur, "port");
def->target.addr = g_new0(virSocketAddr, 1);
@@ -11028,19 +11026,8 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
return -1;
}
-if (portStr == NULL) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("guestfwd channel does "
- "not define a target port"));
-return -1;
-}
-
-if (virStrToLong_ui(portStr, NULL, 10, ) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("Invalid port number: %s"),
- portStr);
+if (virXMLPropUInt(cur, "port", 10, VIR_XML_PROP_REQUIRED, )
< 0)
return -1;
-}
virSocketAddrSetPort(def->target.addr, port);
break;
@@ -11050,18 +11037,12 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
def->target.name = virXMLPropString(cur, "name");
if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
-!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
-(stateStr = virXMLPropString(cur, "state"))) {
-int tmp;
+!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
-if ((tmp = virDomainChrDeviceStateTypeFromString(stateStr)) <=
0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid channel state value '%s'"),
- stateStr);
+if (virXMLPropEnum(cur, "state",
+ virDomainChrDeviceStateTypeFromString,
+ VIR_XML_PROP_NONZERO, >state) < 0)
return -1;
-}
-
-def->state = tmp;
}
break;
}
--
2.26.3
[libvirt PATCH 08/10] virDomainAudioCoreAudioParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 57a54f12ef..a46e64c64a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13046,15 +13046,9 @@ static int
virDomainAudioCoreAudioParse(virDomainAudioIOCoreAudio *def,
xmlNodePtr node)
{
-g_autofree char *bufferCount = virXMLPropString(node, "bufferCount");
-
-if (bufferCount &&
-virStrToLong_ui(bufferCount, NULL, 10,
->bufferCount) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse 'bufferCount' value '%s'"),
bufferCount);
+if (virXMLPropUInt(node, "bufferCount", 10, VIR_XML_PROP_NONE,
+ >bufferCount) < 0)
return -1;
-}
return 0;
}
--
2.26.3
[libvirt PATCH 05/10] virDomainDiskDefGeometryParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attributes `cyls`, `heads` and `secs`.
Allowing negative numbers to be interpreted this way makes no sense for
these attributes.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 48 +++---
1 file changed, 12 insertions(+), 36 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f55117e849..bfcc56ca9e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8815,45 +8815,21 @@ static int
virDomainDiskDefGeometryParse(virDomainDiskDef *def,
xmlNodePtr cur)
{
-g_autofree char *tmp = NULL;
-
-if ((tmp = virXMLPropString(cur, "cyls"))) {
-if (virStrToLong_ui(tmp, NULL, 10, >geometry.cylinders) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid geometry settings (cyls)"));
-return -1;
-}
-VIR_FREE(tmp);
-}
+if (virXMLPropUInt(cur, "cyls", 10, VIR_XML_PROP_NONE,
+ >geometry.cylinders) < 0)
+return -1;
-if ((tmp = virXMLPropString(cur, "heads"))) {
-if (virStrToLong_ui(tmp, NULL, 10, >geometry.heads) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid geometry settings (heads)"));
-return -1;
-}
-VIR_FREE(tmp);
-}
+if (virXMLPropUInt(cur, "heads", 10, VIR_XML_PROP_NONE,
+ >geometry.heads) < 0)
+return -1;
-if ((tmp = virXMLPropString(cur, "secs"))) {
-if (virStrToLong_ui(tmp, NULL, 10, >geometry.sectors) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid geometry settings (secs)"));
-return -1;
-}
-VIR_FREE(tmp);
-}
+if (virXMLPropUInt(cur, "secs", 10, VIR_XML_PROP_NONE,
+ >geometry.sectors) < 0)
+return -1;
-if ((tmp = virXMLPropString(cur, "trans"))) {
-int value;
-if ((value = virDomainDiskGeometryTransTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("invalid translation value '%s'"),
- tmp);
-return -1;
-}
-def->geometry.trans = value;
-}
+if (virXMLPropEnum(cur, "trans", virDomainDiskGeometryTransTypeFromString,
+ VIR_XML_PROP_NONZERO, >geometry.trans) < 0)
+return -1;
return 0;
}
--
2.26.3
[libvirt PATCH 04/10] virDomainDiskDef: Change type of geometry.trans to virDomainDiskGeometryTrans
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 5 +++--
src/conf/domain_conf.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 86680e0cdb..f55117e849 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8845,13 +8845,14 @@ virDomainDiskDefGeometryParse(virDomainDiskDef *def,
}
if ((tmp = virXMLPropString(cur, "trans"))) {
-def->geometry.trans = virDomainDiskGeometryTransTypeFromString(tmp);
-if (def->geometry.trans <= 0) {
+int value;
+if ((value = virDomainDiskGeometryTransTypeFromString(tmp)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("invalid translation value '%s'"),
tmp);
return -1;
}
+def->geometry.trans = value;
}
return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 41e570765e..cf8481f1f6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -549,7 +549,7 @@ struct _virDomainDiskDef {
unsigned int cylinders;
unsigned int heads;
unsigned int sectors;
-int trans; /* enum virDomainDiskGeometryTrans */
+virDomainDiskGeometryTrans trans;
} geometry;
struct {
--
2.26.3
[libvirt PATCH 03/10] virDomainDeviceUSBMasterParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `startport`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 661fa53206..86680e0cdb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6439,18 +6439,11 @@ static int
virDomainDeviceUSBMasterParseXML(xmlNodePtr node,
virDomainDeviceUSBMaster *master)
{
-g_autofree char *startport = NULL;
-
memset(master, 0, sizeof(*master));
-startport = virXMLPropString(node, "startport");
-
-if (startport &&
-virStrToLong_ui(startport, NULL, 10, >startport) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot parse 'startport' attribute"));
+if (virXMLPropUInt(node, "startport", 10, VIR_XML_PROP_NONE,
+ >startport) < 0)
return -1;
-}
return 0;
}
--
2.26.3
[libvirt PATCH 01/10] virDomainHostdevDef: Change type of startupPolicy to virDomainStartupPolicy
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 6 +++---
src/conf/domain_conf.h | 21 ++---
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7044701fac..734fa584a4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6704,14 +6704,14 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
ctxt->node = node;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
-def->startupPolicy =
-virDomainStartupPolicyTypeFromString(startupPolicy);
-if (def->startupPolicy <= 0) {
+int value = virDomainStartupPolicyTypeFromString(startupPolicy);
+if (value <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown startup policy '%s'"),
startupPolicy);
return -1;
}
+def->startupPolicy = value;
}
if ((autoAddress = virXMLPropString(node, "autoAddress")))
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2d5462bb55..41e570765e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -332,6 +332,15 @@ struct _virDomainHostdevCaps {
};
+typedef enum {
+VIR_DOMAIN_STARTUP_POLICY_DEFAULT = 0,
+VIR_DOMAIN_STARTUP_POLICY_MANDATORY,
+VIR_DOMAIN_STARTUP_POLICY_REQUISITE,
+VIR_DOMAIN_STARTUP_POLICY_OPTIONAL,
+
+VIR_DOMAIN_STARTUP_POLICY_LAST
+} virDomainStartupPolicy;
+
/* basic device for direct passthrough */
struct _virDomainHostdevDef {
/* If 'parentnet' is non-NULL it means this host dev was
@@ -343,7 +352,7 @@ struct _virDomainHostdevDef {
virDomainNetDef *parentnet;
int mode; /* enum virDomainHostdevMode */
-int startupPolicy; /* enum virDomainStartupPolicy */
+virDomainStartupPolicy startupPolicy;
bool managed;
bool missing;
bool readonly;
@@ -432,16 +441,6 @@ typedef enum {
VIR_DOMAIN_DISK_IO_LAST
} virDomainDiskIo;
-typedef enum {
-VIR_DOMAIN_STARTUP_POLICY_DEFAULT = 0,
-VIR_DOMAIN_STARTUP_POLICY_MANDATORY,
-VIR_DOMAIN_STARTUP_POLICY_REQUISITE,
-VIR_DOMAIN_STARTUP_POLICY_OPTIONAL,
-
-VIR_DOMAIN_STARTUP_POLICY_LAST
-} virDomainStartupPolicy;
-
-
typedef enum {
VIR_DOMAIN_DEVICE_SGIO_DEFAULT = 0,
VIR_DOMAIN_DEVICE_SGIO_FILTERED,
--
2.26.3
[libvirt PATCH 02/10] virDomainHostdevSubsysUSBDefParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 734fa584a4..661fa53206 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6694,28 +6694,23 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
virDomainHostdevDef *def)
{
virDomainHostdevSubsysUSB *usbsrc = >source.subsys.u.usb;
-g_autofree char *startupPolicy = NULL;
-g_autofree char *autoAddress = NULL;
xmlNodePtr vendorNode;
xmlNodePtr productNode;
xmlNodePtr addressNode;
+virTristateBool autoAddress;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
ctxt->node = node;
-if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
-int value = virDomainStartupPolicyTypeFromString(startupPolicy);
-if (value <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown startup policy '%s'"),
- startupPolicy);
-return -1;
-}
-def->startupPolicy = value;
-}
+if (virXMLPropEnum(node, "startupPolicy",
+ virDomainStartupPolicyTypeFromString,
+ VIR_XML_PROP_NONZERO, >startupPolicy) < 0)
+return -1;
-if ((autoAddress = virXMLPropString(node, "autoAddress")))
-ignore_value(virStringParseYesNo(autoAddress, >autoAddress));
+if (virXMLPropTristateBool(node, "autoAddress", VIR_XML_PROP_NONE,
+ ) < 0)
+return -1;
+usbsrc->autoAddress = autoAddress == VIR_TRISTATE_BOOL_YES;
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized */
--
2.26.3
[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part XI
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainHostdevDef: Change type of startupPolicy to virDomainStartupPolicy virDomainHostdevSubsysUSBDefParseXML: Use virXMLProp* virDomainDeviceUSBMasterParseXML: Use virXMLProp* virDomainDiskDef: Change type of geometry.trans to virDomainDiskGeometryTrans virDomainDiskDefGeometryParse: Use virXMLProp* virDomainChrSourceReconnectDefParseXML: Use virXMLProp* virDomainChrDefParseTargetXML: Use virXMLProp* virDomainAudioCoreAudioParse: Use virXMLProp* virDomainAudioOSSParse: Use virXMLProp* virNodeDevCapPCIDevIommuGroupParseXML: Use virXMLProp* src/conf/domain_conf.c | 168 +--- src/conf/domain_conf.h | 23 +++-- src/conf/node_device_conf.c | 15 +--- 3 files changed, 52 insertions(+), 154 deletions(-) -- 2.26.3
Fwd: Re: [PATCH libvirt v1] tests: add capabilities for QEMU 6.0.0 on s390x
On 5/13/21 12:45 PM, Michal Prívozník wrote: On 5/13/21 11:53 AM, Andrea Bolognani wrote: On Thu, May 13, 2021 at 11:39:24AM +0200, Michal Prívozník wrote: On 5/12/21 6:11 PM, Andrea Bolognani wrote: Overall looks reasonable, but comparing the computed capabilities with those for QEMU 5.2 highlights a couple of changes that I'm not so sure about, specifically --- tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml 2021-05-12 10:52:29.826021415 +0200 +++ tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml 2021-05-12 17:39:47.778445212 +0200 @@ -87,7 +87,6 @@ - @@ -115,7 +114,6 @@ - Because of the former I would expect the s390x-ccw-graphics xml2argv test to fail, but it looks like we don't do a good job at validating the element and that QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW in particular is completely unused. As for the latter there doesn't seem to be any test coverage outside of x86_64, so perhaps it would be a good idea to look into that? pmem was reported by QEMU even if it was later denied. This was fixed by: https://gitlab.com/qemu-project/qemu/-/commit/def835f0da0d153b397071e6bb8f2b46f51f96b4 qemu.git $ git describe --contains def835f0da0d153b397071e6bb8f2b46f51f96b4 v6.0.0-rc0~77^2 Okay for the pmem part - it was reported as available on s390x even though it probably never worked. And it's the same story with virtio-gpu-ccw: https://gitlab.com/qemu-project/qemu/-/commit/adcf33a504de29feb720736051dc32889314c9e6 qemu.git $ git describe --contains adcf33a504de29feb720736051dc32889314c9e6 v6.0.0-rc1~5^2~1 But virtio-gpu-ccw surely should be reported as available on s390x? Perhaps I'm missing something obvious in the commit you pointed me to and I should grab another coffee :) I admit, I don't know. I thought that virtio-gpu was also misleadingly reported as supported. But as you pointed out it's not the case. Shalini, can you chime in? Hello Michal, Andrea, Thank you for the review. I am working on resolving the "virtio-gpu-ccw" availability. I have sent version 2 of QEMU 6.0.0 capabilities patch. The "virtio-gpu-ccw" capability is included in this version. Thank you Shalini C S Michal -- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH] virsh: Fix logic wrt to --current flag in cmdSetmem
On Tue, May 18, 2021 at 03:17:56PM +0200, Michal Prívozník wrote: > On 5/18/21 3:12 PM, Pavel Hrdina wrote: > > On Tue, May 18, 2021 at 03:02:55PM +0200, Ján Tomko wrote: > >> On a Tuesday in 2021, Michal Privoznik wrote: > >>> In my commit of v7.1.0-rc1~376 I've simplified the logic of > >>> handling @flags. My assumption back then was that calling > >>> virDomainSetMemory() is equivalent to > >>> virDomainSetMemoryFlags(flags = 0). But that is not the case, > >>> because it is equivalent to virDomainSetMemoryFlags(flags = > >>> VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old > >>> API. > >>> > >>> Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a > >> > >> before that commit, if the user did not specify any of: > >> config, live, current > >> we used the old API. > >> > >> After this change, the new API will be used for those cases. > > > > The question is if we support using upstream virsh with old libvirt > > where only non-flags API is available. If not we should drop the > > non-flags API usage from virsh completely. > > I think in general we do. In this specific case, > virDomainSetMemoryFlags() API was introduced in v0.9.0 release (which is > 10 years ago). And since we dropped RHEL-7 support recently and the > oldest QEMU we support is from late 2017 our attempt to be that > backwards compatible is questionable IMO. Exactly my question. We have some support matrix where libvirt should be able to compile and run but there is no explicit support policy for using virsh with old libvirt or for obsolete APIs that should not be used. I would vote to use the same matrix as we already have. That would allow us to remove probably all non-flags APIs from virsh code. Pavel > But anyway - with this proposed patch the older API is called in more > cases than it was before I touched the code => compatibility++. > > Michal > signature.asc Description: PGP signature
Re: [libvirt PATCH] meson: Add yajl kludge
On Fri, May 14, 2021 at 11:22:42AM +0200, Andrea Bolognani wrote:
> If this looks familiar, that's because it's literally *the
> same code* that we used to work around *the same issue* in
> readline before 1635dca26f61def3fbf56c70fbbfe514f2b50987 :)
>
> Note that the issue only really affects people building from
> source on Apple Silicon: on Intel, Homebrew installs header
> files under directories that are part of the default search
> path, which explains why our CI pipeline never ran into it.
>
> Signed-off-by: Andrea Bolognani
> ---
> Roman, can you please test this? Thanks! :)
>
Thanks Andrea!
It works fine :)
Reviewed-by: Roman Bolshakov
Tested-by: Roman Bolshakov
> meson.build | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 1f97842319..4f23f9104e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1318,6 +1318,41 @@ endif
> yajl_version = '2.0.3'
> yajl_dep = dependency('yajl', version: '>=' + yajl_version, required:
> get_option('yajl'))
> if yajl_dep.found()
> + # Kludge for yajl include path on non-Linux
> + #
> + # As of 2.1.0, upstream yajl.pc has -I${includedir}/yajl among
> + # its Cflags, which is clearly wrong. This does not affect Linux
> + # because ${includedir} is already part of the default include path,
> + # but on other platforms that's not the case and the result is that
> + # can't be located, causing the build to fail.
> + #
> + # Since upstream development for yajl has stopped years ago, there's
> + # little hope of this issue getting fixed by a new upstream release.
> + # Some non-Linux operating systems such as FreeBSD have elected to
> + # carry a small downstream patch, but in the case of Homebrew on
> + # macOS this approach has been rejected[1] and so we're left with no
> + # choice but to work around the issue ourselves.
> + #
> + # [1] https://github.com/Homebrew/homebrew-core/pull/74516
> + if host_machine.system() != 'linux'
> +pkg_config_prog = find_program('pkg-config')
> +rc = run_command(pkg_config_prog, '--cflags', 'yajl', check: true)
> +cflags = rc.stdout().strip()
> +if cflags.contains('include/yajl')
> + rc = run_command(
> +'python3', '-c',
> +'print("@0@".replace("@1@", "@2@"))'.format(
> + cflags, 'include/yajl', 'include',
> +),
> +check: true,
> + )
> + yajl_dep = declare_dependency(
> +compile_args: rc.stdout().strip().split(),
> +dependencies: [ yajl_dep ],
> + )
> +endif
> + endif
> +
>conf.set('WITH_YAJL', 1)
> endif
>
> --
> 2.26.3
>
[PATCH 1/2] qemu_capabilities: Update QEMU_MIN_* macros
As of b4cbdbe90bbf85eaf687f532d5a52a11e664b781 (and friends) the minimal QEMU version required is 2.11.0. Let's update our QEMU_MIN_* macros to reflect that. Signed-off-by: Michal Privoznik --- src/qemu/qemu_capabilities.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5f0267049b..d3f24feb6a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5229,8 +5229,8 @@ virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCaps *qemuCaps, return 0; } -#define QEMU_MIN_MAJOR 1 -#define QEMU_MIN_MINOR 5 +#define QEMU_MIN_MAJOR 2 +#define QEMU_MIN_MINOR 11 #define QEMU_MIN_MICRO 0 virDomainVirtType -- 2.26.3
[PATCH 2/2] domaincapsdata: Drop expected outputs for old QEMUs
The minimal version of QEMU is 2.11.0 which means we can drop test cases for older versions. Signed-off-by: Michal Privoznik --- .../domaincapsdata/qemu_1.5.3-q35.x86_64.xml | 142 .../domaincapsdata/qemu_1.5.3-tcg.x86_64.xml | 138 tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 142 .../domaincapsdata/qemu_1.6.0-q35.x86_64.xml | 142 .../domaincapsdata/qemu_1.6.0-tcg.x86_64.xml | 138 tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 142 .../domaincapsdata/qemu_1.7.0-q35.x86_64.xml | 142 .../domaincapsdata/qemu_1.7.0-tcg.x86_64.xml | 138 tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 142 .../domaincapsdata/qemu_2.1.1-q35.x86_64.xml | 143 .../domaincapsdata/qemu_2.1.1-tcg.x86_64.xml | 139 tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 143 .../domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 172 --- .../domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 187 .../qemu_2.10.0-virt.aarch64.xml | 151 - tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 145 tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 117 -- tests/domaincapsdata/qemu_2.10.0.s390x.xml| 206 -- tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 172 --- 19 files changed, 2841 deletions(-) delete mode 100644 tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.5.3.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.6.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.6.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.6.0.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.7.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.7.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.7.0.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.1.1-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.1.1-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.1.1.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0-virt.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.ppc64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.s390x.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.x86_64.xml diff --git a/tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml b/tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml deleted file mode 100644 index 76e33a180d..00 --- a/tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml +++ /dev/null @@ -1,142 +0,0 @@ - - /usr/bin/qemu-system-x86_64 - kvm - pc-q35-1.5 - x86_64 - - - - - bios - efi - - - /usr/share/AAVMF/AAVMF_CODE.fd - /usr/share/AAVMF/AAVMF32_CODE.fd - /usr/share/OVMF/OVMF_CODE.fd - -rom -pflash - - -yes -no - - -yes -no - - - - - - -off - - - - - Broadwell - - - Opteron_G5 - Opteron_G4 - Opteron_G3 - Opteron_G2 - Opteron_G1 - Haswell - SandyBridge - Westmere - Nehalem - Penryn - Conroe - n270 - athlon - pentium3 - pentium2 - pentium - 486 - coreduo - kvm32 - qemu32 - kvm64 - core2duo - phenom - qemu64 - - - - - -disk -cdrom -floppy -lun - - -fdc -scsi -virtio -usb -sata - - -virtio - - - - -sdl -vnc -spice - - - - -vga -cirrus -vmvga -qxl -none - - - - -subsystem - - -default -mandatory -requisite -optional - - -usb -pci -scsi - - - -default -vfio - - - - -virtio - - -random -egd - - - - - - - - - - - - diff --git a/tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml b/tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml deleted file mode 100644 index a88af9605a..00 --- a/tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml +++ /dev/null @@ -1,138 +0,0 @@ - - /usr/bin/qemu-system-x86_64 - qemu - pc-i440fx-1.5 - x86_64 - - - - - bios - efi - - - /usr/share/AAVMF/AAVMF_CODE.fd - /usr/share/AAVMF/AAVMF32_CODE.fd -
[PATCH 0/2] Finish min QEMU version bump
*** BLURB HERE *** Michal Prívozník (2): qemu_capabilities: Update QEMU_MIN_* macros domaincapsdata: Drop expected outputs for old QEMUs src/qemu/qemu_capabilities.c | 4 +- .../domaincapsdata/qemu_1.5.3-q35.x86_64.xml | 142 .../domaincapsdata/qemu_1.5.3-tcg.x86_64.xml | 138 tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 142 .../domaincapsdata/qemu_1.6.0-q35.x86_64.xml | 142 .../domaincapsdata/qemu_1.6.0-tcg.x86_64.xml | 138 tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 142 .../domaincapsdata/qemu_1.7.0-q35.x86_64.xml | 142 .../domaincapsdata/qemu_1.7.0-tcg.x86_64.xml | 138 tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 142 .../domaincapsdata/qemu_2.1.1-q35.x86_64.xml | 143 .../domaincapsdata/qemu_2.1.1-tcg.x86_64.xml | 139 tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 143 .../domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 172 --- .../domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 187 .../qemu_2.10.0-virt.aarch64.xml | 151 - tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 145 tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 117 -- tests/domaincapsdata/qemu_2.10.0.s390x.xml| 206 -- tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 172 --- 20 files changed, 2 insertions(+), 2843 deletions(-) delete mode 100644 tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.5.3.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.6.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.6.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.6.0.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.7.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.7.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_1.7.0.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.1.1-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.1.1-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.1.1.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0-q35.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0-tcg.x86_64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0-virt.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.aarch64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.ppc64.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.s390x.xml delete mode 100644 tests/domaincapsdata/qemu_2.10.0.x86_64.xml -- 2.26.3
Re: [PATCH] virsh: Fix logic wrt to --current flag in cmdSetmem
On 5/18/21 3:12 PM, Pavel Hrdina wrote: > On Tue, May 18, 2021 at 03:02:55PM +0200, Ján Tomko wrote: >> On a Tuesday in 2021, Michal Privoznik wrote: >>> In my commit of v7.1.0-rc1~376 I've simplified the logic of >>> handling @flags. My assumption back then was that calling >>> virDomainSetMemory() is equivalent to >>> virDomainSetMemoryFlags(flags = 0). But that is not the case, >>> because it is equivalent to virDomainSetMemoryFlags(flags = >>> VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old >>> API. >>> >>> Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a >> >> before that commit, if the user did not specify any of: >> config, live, current >> we used the old API. >> >> After this change, the new API will be used for those cases. > > The question is if we support using upstream virsh with old libvirt > where only non-flags API is available. If not we should drop the > non-flags API usage from virsh completely. I think in general we do. In this specific case, virDomainSetMemoryFlags() API was introduced in v0.9.0 release (which is 10 years ago). And since we dropped RHEL-7 support recently and the oldest QEMU we support is from late 2017 our attempt to be that backwards compatible is questionable IMO. But anyway - with this proposed patch the older API is called in more cases than it was before I touched the code => compatibility++. Michal
Re: [PATCH] virsh: Fix logic wrt to --current flag in cmdSetmem
On Tue, May 18, 2021 at 03:02:55PM +0200, Ján Tomko wrote: > On a Tuesday in 2021, Michal Privoznik wrote: > > In my commit of v7.1.0-rc1~376 I've simplified the logic of > > handling @flags. My assumption back then was that calling > > virDomainSetMemory() is equivalent to > > virDomainSetMemoryFlags(flags = 0). But that is not the case, > > because it is equivalent to virDomainSetMemoryFlags(flags = > > VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old > > API. > > > > Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a > > before that commit, if the user did not specify any of: > config, live, current > we used the old API. > > After this change, the new API will be used for those cases. The question is if we support using upstream virsh with old libvirt where only non-flags API is available. If not we should drop the non-flags API usage from virsh completely. Pavel signature.asc Description: PGP signature
Re: [PATCH] virsh: Fix logic wrt to --current flag in cmdSetmem
On 5/18/21 3:02 PM, Ján Tomko wrote: > On a Tuesday in 2021, Michal Privoznik wrote: >> In my commit of v7.1.0-rc1~376 I've simplified the logic of >> handling @flags. My assumption back then was that calling >> virDomainSetMemory() is equivalent to >> virDomainSetMemoryFlags(flags = 0). But that is not the case, >> because it is equivalent to virDomainSetMemoryFlags(flags = >> VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old >> API. >> >> Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a > > before that commit, if the user did not specify any of: > config, live, current > we used the old API. > > After this change, the new API will be used for those cases. Will it? I think that with just --live @flags will be VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_AFFECT_LIVE, which is effectively just VIR_DOMAIN_AFFECT_LIVE which means that the old API is used. Am I missing something? Michal
Re: [PATCH] virsh: Fix logic wrt to --current flag in cmdSetmem
On a Tuesday in 2021, Michal Privoznik wrote: In my commit of v7.1.0-rc1~376 I've simplified the logic of handling @flags. My assumption back then was that calling virDomainSetMemory() is equivalent to virDomainSetMemoryFlags(flags = 0). But that is not the case, because it is equivalent to virDomainSetMemoryFlags(flags = VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old API. Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a before that commit, if the user did not specify any of: config, live, current we used the old API. After this change, the new API will be used for those cases. Jano Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961118 Signed-off-by: Michal Privoznik --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) signature.asc Description: PGP signature
Re: [PATCH 0/2] domcaps: Add support for 'filesystem' device
On 5/12/21 7:12 PM, Kristina Hanicova wrote: > > Kristina Hanicova (2): > conf: domcaps: Report device > qemu: capabilities: fill in domcaps > > 107 files changed, 814 insertions(+) > Reviewed-by: Michal Privoznik and pushed. Michal
Re: [PATCH 2/2] qemu: capabilities: fill in domcaps
On 5/12/21 7:12 PM, Kristina Hanicova wrote: > Signed-off-by: Kristina Hanicova > --- > src/qemu/qemu_capabilities.c | 20 +++ > src/qemu/qemu_capabilities.h | 3 +++ > .../domaincapsdata/qemu_1.5.3-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_1.5.3-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_1.6.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_1.6.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_1.7.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_1.7.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_2.1.1-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.1.1-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 7 +++ > .../qemu_2.10.0-virt.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 7 +++ > tests/domaincapsdata/qemu_2.10.0.s390x.xml| 7 +++ > tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_2.11.0.s390x.xml| 7 +++ > tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 7 +++ > .../qemu_2.12.0-virt.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 7 +++ > tests/domaincapsdata/qemu_2.12.0.s390x.xml| 7 +++ > tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.4.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.4.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_2.5.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.5.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_2.6.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.6.0-tcg.x86_64.xml | 7 +++ > .../qemu_2.6.0-virt.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 7 +++ > tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_2.7.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.7.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_2.7.0.s390x.xml | 7 +++ > tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_2.8.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.8.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_2.8.0.s390x.xml | 7 +++ > tests/domaincapsdata/qemu_2.8.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_2.9.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_2.9.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 7 +++ > tests/domaincapsdata/qemu_2.9.0.s390x.xml | 7 +++ > tests/domaincapsdata/qemu_2.9.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_3.0.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 7 +++ > tests/domaincapsdata/qemu_3.0.0.s390x.xml | 7 +++ > tests/domaincapsdata/qemu_3.0.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_3.1.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 7 +++ > tests/domaincapsdata/qemu_3.1.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_4.0.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 7 +++ > .../qemu_4.0.0-virt.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_4.0.0.aarch64.xml | 7 +++ > tests/domaincapsdata/qemu_4.0.0.ppc64.xml | 7 +++ > tests/domaincapsdata/qemu_4.0.0.s390x.xml | 7 +++ > tests/domaincapsdata/qemu_4.0.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_4.1.0-q35.x86_64.xml | 7 +++ > .../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 7 +++ > tests/domaincapsdata/qemu_4.1.0.x86_64.xml| 7 +++ > .../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 8 > .../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 8 > .../qemu_4.2.0-virt.aarch64.xml | 8 >
Re: [PATCH] virsh: Fix logic wrt to --current flag in cmdSetmem
On Tue, May 18, 2021 at 02:30:06PM +0200, Michal Privoznik wrote: > In my commit of v7.1.0-rc1~376 I've simplified the logic of > handling @flags. My assumption back then was that calling > virDomainSetMemory() is equivalent to > virDomainSetMemoryFlags(flags = 0). But that is not the case, > because it is equivalent to virDomainSetMemoryFlags(flags = > VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old > API. > > Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961118 > Signed-off-by: Michal Privoznik > --- > tools/virsh-domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I would add a comment in the code as well to help other developers understand the logic without looking up the commit message. Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
[PATCH] virsh: Fix logic wrt to --current flag in cmdSetmem
In my commit of v7.1.0-rc1~376 I've simplified the logic of
handling @flags. My assumption back then was that calling
virDomainSetMemory() is equivalent to
virDomainSetMemoryFlags(flags = 0). But that is not the case,
because it is equivalent to virDomainSetMemoryFlags(flags =
VIR_DOMAIN_AFFECT_LIVE). Fix the condition that calls the old
API.
Fixes: b5e267e8c59a257652f88d034cb1e0ce1ed4b58a
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961118
Signed-off-by: Michal Privoznik
---
tools/virsh-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0825f82522..bd9325a1d8 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9025,7 +9025,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
}
kibibytes = VIR_DIV_UP(bytes, 1024);
-if (flags == 0) {
+if (flags == VIR_DOMAIN_AFFECT_LIVE) {
if (virDomainSetMemory(dom, kibibytes) != 0)
ret = false;
} else {
--
2.26.3
Re: [PATCH] qemusecuritytest: Honour EXIT_AM_SKIP
On a Tuesday in 2021, Michal Privoznik wrote: There is a case where qemusecuritytest is skipped - on MacOS and MinGW. In such case, EXIT_AM_SKIP should be returned. However, my recent patch of 5d99b157bc completely missed that and made the test return EXIT_FAILURE even though the test exited early without performing any test case. Signed-off-by: Michal Privoznik --- tests/qemusecuritytest.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH] qemusecuritytest: Honour EXIT_AM_SKIP
There is a case where qemusecuritytest is skipped - on MacOS and
MinGW. In such case, EXIT_AM_SKIP should be returned. However,
my recent patch of 5d99b157bc completely missed that and made the
test return EXIT_FAILURE even though the test exited early
without performing any test case.
Signed-off-by: Michal Privoznik
---
tests/qemusecuritytest.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tests/qemusecuritytest.c b/tests/qemusecuritytest.c
index f7186700c4..a7e87fdf8f 100644
--- a/tests/qemusecuritytest.c
+++ b/tests/qemusecuritytest.c
@@ -143,15 +143,13 @@ mymain(void)
#endif
int ret = 0;
+if (!virSecurityXATTRNamespaceDefined())
+return EXIT_AM_SKIP;
+
if (virInitialize() < 0 ||
qemuTestDriverInit() < 0)
return -1;
-if (!virSecurityXATTRNamespaceDefined()) {
-ret = EXIT_AM_SKIP;
-goto cleanup;
-}
-
/* Now fix the secdriver */
virObjectUnref(driver.securityManager);
--
2.26.3
[PATCH] conf: Report alias name of the detached device in error
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942367
Signed-off-by: Kristina Hanicova
---
src/conf/domain_conf.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7044701fac..e21b9fb946 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15781,38 +15781,45 @@ virDomainNetFindIdx(virDomainDef *def,
virDomainNetDef *net)
if (matchidx < 0) {
if (MACAddrSpecified && PCIAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device matching MAC address %s found on "
+ _("no device matching MAC address %s and alias %s
found on "
VIR_PCI_DEVICE_ADDRESS_FMT),
virMacAddrFormat(>mac, mac),
+ NULLSTR(net->info.alias),
net->info.addr.pci.domain,
net->info.addr.pci.bus,
net->info.addr.pci.slot,
net->info.addr.pci.function);
} else if (MACAddrSpecified && CCWAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device matching MAC address %s found on "
+ _("no device matching MAC address %s and alias %s
found on "
VIR_CCW_DEVICE_ADDRESS_FMT),
virMacAddrFormat(>mac, mac),
+ NULLSTR(net->info.alias),
net->info.addr.ccw.cssid,
net->info.addr.ccw.ssid,
net->info.addr.ccw.devno);
} else if (PCIAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device found on " VIR_PCI_DEVICE_ADDRESS_FMT),
+ _("no device matching alias %s found on "
+ VIR_PCI_DEVICE_ADDRESS_FMT),
+ NULLSTR(net->info.alias),
net->info.addr.pci.domain,
net->info.addr.pci.bus,
net->info.addr.pci.slot,
net->info.addr.pci.function);
} else if (CCWAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device found on " VIR_CCW_DEVICE_ADDRESS_FMT),
+ _("no device matching alias %s found on "
+ VIR_CCW_DEVICE_ADDRESS_FMT),
+ NULLSTR(net->info.alias),
net->info.addr.ccw.cssid,
net->info.addr.ccw.ssid,
net->info.addr.ccw.devno);
} else if (MACAddrSpecified) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device matching MAC address %s found"),
- virMacAddrFormat(>mac, mac));
+ _("no device matching MAC address %s and alias %s
found"),
+ virMacAddrFormat(>mac, mac),
+ NULLSTR(net->info.alias));
} else {
virReportError(VIR_ERR_DEVICE_MISSING, "%s",
_("no matching device found"));
--
2.31.1
Re: [libvirt PATCH v2 0/7] Enable sanitizers
Ping. On Thu, 2021-05-06 at 17:08 +0200, Tim Wiederhake wrote: > This series enables and adds AddressSanitizer and > UndefinedBehaviorSanitizer > builds to the CI. > > See: > https://clang.llvm.org/docs/AddressSanitizer.html and > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html > > These sanitizers already found some issues in libvirt, e.g. > 4eb7c621985dad4de911ec394ac628bd1a5b29ab, > 1294de209cee6643511265c7e2d4283c047cf652, > 8b8c91f487592c6c067847ca59dde405ca17573f, or > 1c34211c22de28127a509edbf2cf2f44cb0d891e. > > There exist two more relevant sanitizers, ThreadSanitizer and > MemorySanitizer. > Unfortunately, those two require an instrumented build of all > dependencies, > including libc, to work correctly. > > Note that clang and gcc have different implementations of these > sanitizers, > hence the introduction of two new jobs to the CI. The latter one > issues a > warning about the use of LD_PRELOAD in `virTestMain`, which in this > particular case can be safely ignored by setting `ASAN_OPTIONS` to > verify_asan_link_order=0` for the gcc build. > > Changes since V1: > > Incorporated changes suggested by Pavel, except for #6 (now #7): The > statement > in > https://listman.redhat.com/archives/libvir-list/2021-May/msg00070.html > on > the sanitizers working with Fedora 33 is wrong, I was fooled by > caching. The > bug described there is present in Fedora 33, 34, and Rawhide. > > Cheers, > Tim > > Tim Wiederhake (7): > meson: Allow larger stack frames when instrumenting > meson: Allow undefined symbols when sanitizers are enabled > tests: virfilemock: realpath: Allow non-null second parameter > openvz: Add missing symbols to libvirt_openvz.syms > tests: openvzutilstest: Remove duplicate linking with > libvirt_openvz.a > virt-aa-helper: Remove duplicate linking with src/datatypes.o > ci: Enable address and undefined behavior sanitizers > > .gitlab-ci.yml| 35 +++ > build-aux/syntax-check.mk | 2 +- > meson.build | 14 ++ > src/libvirt_openvz.syms | 2 ++ > src/security/meson.build | 1 - > tests/meson.build | 2 +- > tests/virfilemock.c | 20 > 7 files changed, 61 insertions(+), 15 deletions(-) > > -- > 2.26.3 > >
Re: [PATCH] tests: Replace deprecated ASN1 code
On 5/18/21 3:19 AM, Luke Yue wrote: > This fixes compiler warnings when building with libtasn1 4.17.0. > > Signed-off-by: Luke Yue > --- > tests/pkix_asn1_tab.c| 2 +- > tests/virnettlshelpers.c | 12 ++-- > 2 files changed, 7 insertions(+), 7 deletions(-) Reviewed-by: Michal Privoznik and pushed. Michal
Re: [PATCH] tests: libxl: Mock xs_open and xs_close
On 5/18/21 12:28 AM, Jim Fehlig wrote:
> The Xen-related unit tests are failing against the recently released
> Xen 4.15. Xen commit 90c9f9f4dd changed the implementation of
> libxl_ctx_alloc to use xs_open instead of xs_daemon_open. libvirt has
> already mocked xs_daemon-{open,close} and others to allow using libxl
> in confined build environments. This patch adds xs_{open,close} to the
> list of functions mocked in libxlmock.c
>
> https://github.com/xen-project/xen/commit/90c9f9f4ddd55e11be0506bff10c6237507c6e0d
>
> Signed-off-by: Jim Fehlig
> ---
> tests/libxlmock.c | 11 +++
> 1 file changed, 11 insertions(+)
Reviewed-by: Michal Privoznik
Michal
