date:20210920

RE: [PATCH 0/5] qemu: add virtio-blk queue-size option

2021-09-20 Thread Hiroki Narukawa

Hello,

Can I know how is the status of this patchset? In this patchset all reviews I 
received are applied, and this is ready for review.


-Original Message-
From: 成川 弘樹  
Sent: Thursday, September 9, 2021 12:35 PM
To: libvir-list@redhat.com
Cc: 大岩 朗 ; 成川 弘樹 
Subject: [PATCH 0/5] qemu: add virtio-blk queue-size option

This is resubmit of "qemu: add virtio-blk queue-size option" after following 
the review.

The option "queue-size" in virtio-blk was added in qemu-2.12.0, and default 
value increased from qemu-5.0.0.
However, increasing this value may lead to drop of random access performance.
This is configurable value, so we want to use it via libvirt.

Hiroki Narukawa (5):
  qemu: Make disk-virtio-queues tests use DO_TEST_CAPS_LATEST
  qemu: add disk queue count ABI stability check
  qemu: add queue_size option to disk
  qemu: add QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE capability
  qemu: add virtio-blk queue-size option

 docs/formatdomain.rst |  4 +-
 docs/schemas/domaincommon.rng |  5 +++
 src/conf/domain_conf.c| 20 ++
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_command.c   |  3 ++
 src/qemu/qemu_validate.c  |  7 
 .../caps_2.12.0.aarch64.xml   |  1 +
 .../caps_2.12.0.ppc64.xml |  1 +
 .../caps_2.12.0.s390x.xml |  1 +
 .../caps_2.12.0.x86_64.xml|  1 +
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 +  
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  1 +
 .../caps_3.0.0.x86_64.xml |  1 +
 .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 +
 .../caps_3.1.0.x86_64.xml |  1 +
 .../caps_4.0.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |  1 +
 .../caps_4.0.0.riscv32.xml|  1 +
 .../caps_4.0.0.riscv64.xml|  1 +
 .../qemucapabilitiesdata/caps_4.0.0.s390x.xml |  1 +
 .../caps_4.0.0.x86_64.xml |  1 +
 .../caps_4.1.0.x86_64.xml |  1 +
 .../caps_4.2.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |  1 +  
.../qemucapabilitiesdata/caps_4.2.0.s390x.xml |  1 +
 .../caps_4.2.0.x86_64.xml |  1 +
 .../caps_5.0.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |  1 +
 .../caps_5.0.0.riscv64.xml|  1 +
 .../caps_5.0.0.x86_64.xml |  1 +
 .../caps_5.1.0.x86_64.xml |  1 +
 .../caps_5.2.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml |  1 +
 .../caps_5.2.0.riscv64.xml|  1 +
 .../qemucapabilitiesdata/caps_5.2.0.s390x.xml |  1 +
 .../caps_5.2.0.x86_64.xml |  1 +
 .../caps_6.0.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_6.0.0.s390x.xml |  1 +
 .../caps_6.0.0.x86_64.xml |  1 +
 .../caps_6.1.0.x86_64.xml |  1 +
 .../disk-virtio-queue-size.args   | 35 +
 .../disk-virtio-queue-size.x86_64-latest.args | 35 +
 .../disk-virtio-queue-size.xml| 38 +++
 .../qemuxml2argvdata/disk-virtio-queues.args  | 20 ++
 .../disk-virtio-queues.x86_64-latest.args | 35 +
 tests/qemuxml2argvdata/disk-virtio-queues.xml |  5 ++-
 tests/qemuxml2argvtest.c  |  4 +-
 .../disk-virtio-queue-size.x86_64-latest.xml  | 38 +++
 .../disk-virtio-queue-size.xml| 35 +
 .../disk-virtio-queues.x86_64-latest.xml  |  1 +
 tests/qemuxml2xmltest.c   |  3 +-
 53 files changed, 314 insertions(+), 12 deletions(-)  create mode 100644 
tests/qemuxml2argvdata/disk-virtio-queue-size.args
 create mode 100644 
tests/qemuxml2argvdata/disk-virtio-queue-size.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.xml
 create mode 100644 tests/qemuxml2argvdata/disk-virtio-queues.x86_64-latest.args
 create mode 100644 
tests/qemuxml2xmloutdata/disk-virtio-queue-size.x86_64-latest.xml
 create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-queue-size.xml
 create mode 12 
tests/qemuxml2xmloutdata/disk-virtio-queues.x86_64-latest.xml

--
2.17.1

[libvirt PATCH 2/2] virNWFilterRuleDefFixup: Replace macro with function

2021-09-20 Thread Tim Wiederhake

Signed-off-by: Tim Wiederhake 
---
 src/conf/nwfilter_conf.c | 284 ---
 1 file changed, 143 insertions(+), 141 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index a3109962af..4c4e31d5cd 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2187,71 +2187,74 @@ virNWFilterRuleValidate(virNWFilterRuleDef *rule)
 
 
 static void
-virNWFilterRuleDefFixup(virNWFilterRuleDef *rule)
+copy_neg_sign(nwItemDesc *to, const nwItemDesc *from)
 {
-#define COPY_NEG_SIGN(A, B) \
-(A).flags = ((A).flags & ~NWFILTER_ENTRY_ITEM_FLAG_IS_NEG) | \
-((B).flags &  NWFILTER_ENTRY_ITEM_FLAG_IS_NEG);
+to->flags = (to->flags & ~NWFILTER_ENTRY_ITEM_FLAG_IS_NEG) |
+(from->flags & NWFILTER_ENTRY_ITEM_FLAG_IS_NEG);
+}
 
+static void
+virNWFilterRuleDefFixup(virNWFilterRuleDef *rule)
+{
 switch (rule->prtclType) {
 case VIR_NWFILTER_RULE_PROTOCOL_MAC:
-COPY_NEG_SIGN(rule->p.ethHdrFilter.ethHdr.dataSrcMACMask,
-  rule->p.ethHdrFilter.ethHdr.dataSrcMACAddr);
-COPY_NEG_SIGN(rule->p.ethHdrFilter.ethHdr.dataDstMACMask,
-  rule->p.ethHdrFilter.ethHdr.dataDstMACAddr);
+copy_neg_sign(>p.ethHdrFilter.ethHdr.dataSrcMACMask,
+  >p.ethHdrFilter.ethHdr.dataSrcMACAddr);
+copy_neg_sign(>p.ethHdrFilter.ethHdr.dataDstMACMask,
+  >p.ethHdrFilter.ethHdr.dataDstMACAddr);
 break;
 
 case VIR_NWFILTER_RULE_PROTOCOL_VLAN:
-COPY_NEG_SIGN(rule->p.vlanHdrFilter.ethHdr.dataSrcMACMask,
-  rule->p.vlanHdrFilter.ethHdr.dataSrcMACAddr);
-COPY_NEG_SIGN(rule->p.vlanHdrFilter.ethHdr.dataDstMACMask,
-  rule->p.vlanHdrFilter.ethHdr.dataDstMACAddr);
+copy_neg_sign(>p.vlanHdrFilter.ethHdr.dataSrcMACMask,
+  >p.vlanHdrFilter.ethHdr.dataSrcMACAddr);
+copy_neg_sign(>p.vlanHdrFilter.ethHdr.dataDstMACMask,
+  >p.vlanHdrFilter.ethHdr.dataDstMACAddr);
 break;
 
 case VIR_NWFILTER_RULE_PROTOCOL_STP:
-COPY_NEG_SIGN(rule->p.stpHdrFilter.ethHdr.dataSrcMACMask,
-  rule->p.stpHdrFilter.ethHdr.dataSrcMACAddr);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootPriHi,
-  rule->p.stpHdrFilter.dataRootPri);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootAddrMask,
-  rule->p.stpHdrFilter.dataRootAddr);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootCostHi,
-  rule->p.stpHdrFilter.dataRootCost);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataSndrPrioHi,
-  rule->p.stpHdrFilter.dataSndrPrio);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataSndrAddrMask,
-  rule->p.stpHdrFilter.dataSndrAddr);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataPortHi,
-  rule->p.stpHdrFilter.dataPort);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataAgeHi,
-  rule->p.stpHdrFilter.dataAge);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataMaxAgeHi,
-  rule->p.stpHdrFilter.dataMaxAge);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataHelloTimeHi,
-  rule->p.stpHdrFilter.dataHelloTime);
-COPY_NEG_SIGN(rule->p.stpHdrFilter.dataFwdDelayHi,
-  rule->p.stpHdrFilter.dataFwdDelay);
+copy_neg_sign(>p.stpHdrFilter.ethHdr.dataSrcMACMask,
+  >p.stpHdrFilter.ethHdr.dataSrcMACAddr);
+copy_neg_sign(>p.stpHdrFilter.dataRootPriHi,
+  >p.stpHdrFilter.dataRootPri);
+copy_neg_sign(>p.stpHdrFilter.dataRootAddrMask,
+  >p.stpHdrFilter.dataRootAddr);
+copy_neg_sign(>p.stpHdrFilter.dataRootCostHi,
+  >p.stpHdrFilter.dataRootCost);
+copy_neg_sign(>p.stpHdrFilter.dataSndrPrioHi,
+  >p.stpHdrFilter.dataSndrPrio);
+copy_neg_sign(>p.stpHdrFilter.dataSndrAddrMask,
+  >p.stpHdrFilter.dataSndrAddr);
+copy_neg_sign(>p.stpHdrFilter.dataPortHi,
+  >p.stpHdrFilter.dataPort);
+copy_neg_sign(>p.stpHdrFilter.dataAgeHi,
+  >p.stpHdrFilter.dataAge);
+copy_neg_sign(>p.stpHdrFilter.dataMaxAgeHi,
+  >p.stpHdrFilter.dataMaxAge);
+copy_neg_sign(>p.stpHdrFilter.dataHelloTimeHi,
+  >p.stpHdrFilter.dataHelloTime);
+copy_neg_sign(>p.stpHdrFilter.dataFwdDelayHi,
+  >p.stpHdrFilter.dataFwdDelay);
 break;
 
 case VIR_NWFILTER_RULE_PROTOCOL_IP:
-COPY_NEG_SIGN(rule->p.ipHdrFilter.ipHdr.dataSrcIPMask,
-  rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr);
-COPY_NEG_SIGN(rule->p.ipHdrFilter.ipHdr.dataDstIPMask,
-  rule->p.ipHdrFilter.ipHdr.dataDstIPAddr);
+

[libvirt PATCH 0/2] Random improvements to work around a build system issue

2021-09-20 Thread Tim Wiederhake

This is an alternative to
https://listman.redhat.com/archives/libvir-list/2021-September/msg00522.html.

When libvirt is build:
* with sanitizers enabled,
* buildtype explicitly set to "debug",
* on clang,

the build fails with:

../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616
bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=]
virNWFilterRuleDefFixup(virNWFilterRuleDef *rule)
^
1 error generated.

../src/conf/domain_conf.c:19514:1: error: stack frame size of 8312
bytes in function 'virDomainDefParseXML' [-Werror,-Wframe-larger-than=]
virDomainDefParseXML(xmlXPathContextPtr ctxt,
^
1 error generated.

Note that this does not happen when "-Dbuildtype" is not specified, even
though "debug" is the default build type.

The patches in this series happen to make these errors go away.

Regards,
Tim

Tim Wiederhake (2):
  virDomainDefParseXML: Use automatic memory management
  virNWFilterRuleDefFixup: Replace macro with function

 src/conf/domain_conf.c   | 208 ++--
 src/conf/nwfilter_conf.c | 284 ---
 2 files changed, 245 insertions(+), 247 deletions(-)

-- 
2.31.1

[libvirt PATCH 1/2] virDomainDefParseXML: Use automatic memory management

2021-09-20 Thread Tim Wiederhake

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 208 -
 1 file changed, 102 insertions(+), 106 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c32609431..cdbc66d9fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19518,27 +19518,27 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
 xmlNodePtr node = NULL;
 size_t i, j;
 int n;
-virDomainDef *def;
 bool uuid_generated = false;
 bool usb_none = false;
 g_autofree xmlNodePtr *nodes = NULL;
 g_autofree char *tmp = NULL;
+g_autoptr(virDomainDef) def = NULL;
 
 if (!(def = virDomainDefNew(xmlopt)))
 return NULL;
 
 if (virDomainDefParseIDs(def, ctxt, flags, _generated) < 0)
-goto error;
+return NULL;
 
 if (virDomainDefParseCaps(def, ctxt, xmlopt) < 0)
-goto error;
+return NULL;
 
 /* Extract short description of domain (title) */
 def->title = virXPathString("string(./title[1])", ctxt);
 if (def->title && strchr(def->title, '\n')) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Domain title can't contain newlines"));
-goto error;
+return NULL;
 }
 
 /* Extract documentation if present */
@@ -19548,40 +19548,40 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
  * late, so devices can refer to this for defaults */
 if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL)) {
 if (virSecurityLabelDefsParseXML(def, ctxt, xmlopt, flags) == -1)
-goto error;
+return NULL;
 }
 
 if (virDomainDefParseMemory(def, ctxt) < 0)
-goto error;
+return NULL;
 
 if (virDomainDefTunablesParse(def, ctxt, xmlopt, flags) < 0)
-goto error;
+return NULL;
 
 if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, >cpu,
   false) < 0)
-goto error;
+return NULL;
 
 if (virDomainNumaDefParseXML(def->numa, ctxt) < 0)
-goto error;
+return NULL;
 
 if (virDomainNumaGetCPUCountTotal(def->numa) > 
virDomainDefGetVcpusMax(def)) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Number of CPUs in  exceeds the"
  "  count"));
-goto error;
+return NULL;
 }
 
 if (virDomainNumaGetMaxCPUID(def->numa) >= virDomainDefGetVcpusMax(def)) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("CPU IDs in  exceed the  count"));
-goto error;
+return NULL;
 }
 
 if (virDomainNumatuneParseXML(def->numa,
   def->placement_mode ==
   VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
   ctxt) < 0)
-goto error;
+return NULL;
 
 if (virDomainNumatuneHasPlacementAuto(def->numa) &&
 !def->cpumask && !virDomainDefHasVcpuPin(def) &&
@@ -19592,38 +19592,38 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
 if ((n = virXPathNodeSet("./resource", ctxt, )) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot extract resource nodes"));
-goto error;
+return NULL;
 }
 
 if (n > 1) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("only one resource element is supported"));
-goto error;
+return NULL;
 }
 
 if (n &&
 !(def->resource = virDomainResourceDefParse(nodes[0], ctxt)))
-goto error;
+return NULL;
 VIR_FREE(nodes);
 
 if (virDomainFeaturesDefParse(def, ctxt) < 0)
-goto error;
+return NULL;
 
 if (virDomainDefLifecycleParse(def, ctxt) < 0)
-goto error;
+return NULL;
 
 if (virDomainPerfDefParseXML(def, ctxt) < 0)
-goto error;
+return NULL;
 
 if (virDomainDefClockParse(def, ctxt) < 0)
-goto error;
+return NULL;
 
 if (virDomainDefParseBootOptions(def, ctxt) < 0)
-goto error;
+return NULL;
 
 /* analysis of the disk devices */
 if ((n = virXPathNodeSet("./devices/disk", ctxt, )) < 0)
-goto error;
+return NULL;
 
 for (i = 0; i < n; i++) {
 virDomainDiskDef *disk = virDomainDiskDefParseXML(xmlopt,
@@ -19631,27 +19631,27 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
   ctxt,
   flags);
 if (!disk)
-goto error;
+return NULL;
 
 virDomainDiskInsert(def, disk);
 }
 VIR_FREE(nodes);
 
 if (virDomainDefControllersParse(def, ctxt, xmlopt, flags, _none) < 0)
-goto error;
+return NULL;
 
 /* analysis of the resource leases */
 if ((n = virXPathNodeSet("./devices/lease", ctxt, )) < 0) {

question: about the bug: current master had lost the ability "Cancel disk mirrors after libvirtd restart"

2021-09-20 Thread wangjie (P)

bug reproduce process:
1、perform migrateToURI3.
2、kill libvirtd when enter memory migration phase，and restart libvirtd.
3、perform migrateToURI3 again and again，migrateToURI3 will fail forever with 
err-msg "Requested operation is not valid: domain has active block job"


I found the reasion which trigger the bug as follow:

1、the qemuBlockJobData is not persistent when libvirtd restart，so the job which 
return from qemuBlockJobDiskGetJob while always NULL, so 
qemuMigrationSrcNBDCopyCancel will not be taken.

2、calltrace:
qemuProcessReconnect
->qemuProcessRecoverJob
  ->qemuProcessRecoverMigrationOut
->qemuMigrationSrcCancel

3、code as follow:
qemuMigrationSrcCancel(virQEMUDriver *driver,
   virDomainObj *vm)
{
... ...
for (i = 0; i < vm->def->ndisks; i++) {
virDomainDiskDef *disk = vm->def->disks[i];
qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
qemuBlockJobData *job;

if (!(job = qemuBlockJobDiskGetJob(disk)) ||  //the job is 
always NULL !!!
!qemuBlockJobIsRunning(job))
diskPriv->migrating = false;

if (diskPriv->migrating) {
qemuBlockJobSyncBegin(job);
storage = true;
}

virObjectUnref(job);
}
... ...

if (storage &&
qemuMigrationSrcNBDCopyCancel(driver, vm, true,
  QEMU_ASYNC_JOB_NONE, NULL) < 0)
return -1;
... ...
}


4、I think current master had lost the ability of the followed patch:
http://10.175.124.40/cgit/cgit.cgi/code.huawei.com/libvirt.git/commit/?id=e8f263e0d006390c3764aaa07093b2d174b61379


can you give some suggestions to fix it?

Re: question: about the bug: current master had lost the ability "Cancel disk mirrors after libvirtd restart"

2021-09-20 Thread wangjie (P)

I think current master had lost the ability of the followed patch:
https://github.com/libvirt/libvirt/commit/e8f263e0d006390c3764aaa07093b2d174b61379

On 2021/9/21 0:52, wangjie (P) wrote:
> bug reproduce process:
> 1、perform migrateToURI3.
> 2、kill libvirtd when enter memory migration phase，and restart libvirtd.
> 3、perform migrateToURI3 again and again，migrateToURI3 will fail forever with 
> err-msg "Requested operation is not valid: domain has active block job"
> 
> 
> I found the reasion which trigger the bug as follow:
> 
> 1、the qemuBlockJobData is not persistent when libvirtd restart，so the job 
> which return from qemuBlockJobDiskGetJob while always NULL, so 
> qemuMigrationSrcNBDCopyCancel will not be taken.
> 
> 2、calltrace:
> qemuProcessReconnect
> ->qemuProcessRecoverJob
>   ->qemuProcessRecoverMigrationOut
> ->qemuMigrationSrcCancel
> 
> 3、code as follow:
> qemuMigrationSrcCancel(virQEMUDriver *driver,
>virDomainObj *vm)
> {
> ... ...
> for (i = 0; i < vm->def->ndisks; i++) {
> virDomainDiskDef *disk = vm->def->disks[i];
> qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> qemuBlockJobData *job;
> 
> if (!(job = qemuBlockJobDiskGetJob(disk)) ||  //the job 
> is always NULL !!!
> !qemuBlockJobIsRunning(job))
> diskPriv->migrating = false;
> 
> if (diskPriv->migrating) {
> qemuBlockJobSyncBegin(job);
> storage = true;
> }
> 
> virObjectUnref(job);
> }
> ... ...
> 
> if (storage &&
> qemuMigrationSrcNBDCopyCancel(driver, vm, true,
>   QEMU_ASYNC_JOB_NONE, NULL) < 0)
> return -1;
> ... ...
> }
> 
> 
> 4、I think current master had lost the ability of the followed patch:
> http://10.175.124.40/cgit/cgit.cgi/code.huawei.com/libvirt.git/commit/?id=e8f263e0d006390c3764aaa07093b2d174b61379
> 
> 
> can you give some suggestions to fix it?
> 
> 
> 
>

Re: [PATCH 0/4] virsh: Follow up improvements

2021-09-20 Thread Ján Tomko


On a Friday in 2021, Michal Privoznik wrote:

These are the fixes I spotted when reviewing Peter's patchset earlier.

Michal Prívozník (4):
 virsh: Provide local path completer for screenshot --file
 virsh: Provide local path completer for vol-download --file
 vsh: Extend checks for aliased commands
 vsh: Ensure that bool --options don't have completer

tools/virsh-domain.c |  1 +
tools/virsh-volume.c |  6 +-
tools/vsh.c  | 30 +-
3 files changed, 31 insertions(+), 6 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH 00/14] qemu: more 'query-command-line-options' cleanups

2021-09-20 Thread Ján Tomko


On a Monday in 2021, Peter Krempa wrote:

In a private conversation Markus dug out the history of certain of the
flags we probe via 'query-command-line-options'. I already had some
patches for this but without the history or justification.

To prevent us going through this again I've decided to send some more
removal of capability bits based on stuff that all qemu's we support
have.

Peter Krempa (14):
 qemu: command: Always assume 'QEMU_CAPS_BOOT_STRICT'
 qemu: capabilities: Retire QEMU_CAPS_BOOT_STRICT
 tests: qemuxml2argv: Remove negative case for 'reboot-timeout-enabled'
 qemu: Always assume QEMU_CAPS_REBOOT_TIMEOUT
 qemu: capabilities: Retire QEMU_CAPS_REBOOT_TIMEOUT
 qemuxml2argvtest: Remove negative case for
   'boot-menu-enable-with-timeout'
 qemu: Always assume QEMU_CAPS_SPLASH_TIMEOUT
 qemu: capabilities: Retire QEMU_CAPS_SPLASH_TIMEOUT
 qemu: Always assume QEMU_CAPS_MEM_MERGE
 qemu: capabilities:  QEMU_CAPS_MEM_MERGE
 qemu: capabilities: Assume QEMU_CAPS_AES_KEY_WRAP and
   QEMU_CAPS_DEA_KEY_WRAP for s390 only
 qemu: capabilities: Assume QEMU_CAPS_LOADPARM for s390 only
 qemu: Assume QEMU_CAPS_FW_CFG
 qemu: capabilities: Retire QEMU_CAPS_FW_CFG

src/qemu/qemu_capabilities.c  | 27 ++--
src/qemu/qemu_capabilities.h  | 10 ++---
src/qemu/qemu_command.c   | 10 ++---
src/qemu/qemu_validate.c  | 42 ++-
.../caps_2.11.0.s390x.xml |  5 ---


[...]


tests/qemuxml2argvdata/watchdog-dump.args |  1 +
.../qemuxml2argvdata/watchdog-injectnmi.args  |  1 +
tests/qemuxml2argvdata/watchdog.args  |  1 +
tests/qemuxml2argvdata/x86-kvm-32-on-64.args  |  1 +
tests/qemuxml2argvtest.c  | 17 +++-
tests/qemuxml2xmltest.c   |  8 ++--
609 files changed, 596 insertions(+), 390 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/boot-menu-enable-with-timeout.err
delete mode 100644 tests/qemuxml2argvdata/reboot-timeout-enabled.err



Thank you.

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH 01/14] qemu: command: Always assume 'QEMU_CAPS_BOOT_STRICT'

2021-09-20 Thread Ján Tomko


On a Monday in 2021, Peter Krempa wrote:

Added by c8a6ae8bb9 in qemu-v1.5.0 and can't be compiled out. Assume
that it's present and fix all fake-caps tests.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_command.c| 10 +++---
tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args  |  1 +
tests/qemuxml2argvdata/aarch64-acpi-uefi.args  |  1 +


[...]


tests/qemuxml2argvdata/watchdog-injectnmi.args |  1 +
tests/qemuxml2argvdata/watchdog.args   |  1 +
tests/qemuxml2argvdata/x86-kvm-32-on-64.args   |  1 +
tests/qemuxml2argvtest.c   |  1 -
564 files changed, 565 insertions(+), 25 deletions(-)



[...]


diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 74af93b08f..74a9f5a0ee 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1082,7 +1082,6 @@ mymain(void)
DO_TEST("boot-complex",
QEMU_CAPS_VIRTIO_BLK_SCSI);
DO_TEST("boot-strict",
-QEMU_CAPS_BOOT_STRICT,
QEMU_CAPS_VIRTIO_BLK_SCSI);



The test can be removed completely because it's identical to
"boot-complex" after this change

Jano


/* Simplest possible , all supported with ENV */
--
2.31.1



signature.asc
Description: PGP signature

[libvirt PATCH] scripts: fix API parsing of *** pointers

2021-09-20 Thread Daniel P . Berrangé

The currrent generated API contains *** pointer types with bogus
whitespace in the middle:

  

because the tokenizer only tries to merge 2 distinct '*' together.
This refactors the code to merge an arbitrary number, resulting
in

  

Signed-off-by: Daniel P. Berrangé 
---
 scripts/apibuild.py | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/scripts/apibuild.py b/scripts/apibuild.py
index b94c0f6c09..722fd33f0e 100755
--- a/scripts/apibuild.py
+++ b/scripts/apibuild.py
@@ -603,13 +603,12 @@ class CLexer:
 i = i + 3
 continue
 
-j = i + 1
-if j < nline and line[j] in "+-*><=/%&!|":
-self.tokens.append(('op', line[i:j + 1]))
-i = j + 1
-else:
-self.tokens.append(('op', line[i]))
-i = i + 1
+j = i
+while (j + 1) < nline and line[j+1] in "+-*><=/%&!|":
+j = j + 1
+
+self.tokens.append(('op', line[i:j+1]))
+i = j + 1
 continue
 s = i
 while i < nline:
-- 
2.31.1

Re: [PATCH 0/4] Various cleanups

2021-09-20 Thread Ján Tomko


On a Monday in 2021, Peter Krempa wrote:

Few patches from old branches.

Peter Krempa (4):
 util: virdevmapper: Sanitize use of macros for buffer size
 virDevMapperGetTargets: Use a linked list as return type
 util: virstring: Remove unused 'virStringListMerge'
 qemuMonitorJSONGetStatus: Refactor cleanup

src/libvirt_private.syms |  1 -
src/qemu/qemu_cgroup.c   | 11 
src/qemu/qemu_monitor_json.c | 18 +
src/qemu/qemu_namespace.c|  9 +++
src/util/virdevmapper.c  | 49 +---
src/util/virdevmapper.h  |  2 +-
src/util/virstring.c | 35 --
src/util/virstring.h |  3 ---
tests/qemuhotplugmock.c  | 10 +++-
9 files changed, 43 insertions(+), 95 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH 2/2] selinux: Don't ignore ENOENT in Permissive mode

2021-09-20 Thread Ján Tomko


On a Monday in 2021, Michal Privoznik wrote:

In selinux driver there's virSecuritySELinuxSetFileconImpl()
which is responsible for actual setting of SELinux label on given
file and handling possible failures. In fhe failure handling code
we decide whether failure is fatal or not. But there is a bug:
depending on SELinux mode (Permissive vs. Enforcing) the ENOENT
is either ignored or considered fatal.



This not correct - ENOENT
must always be fatal - QEMU will fail opening it anyways.

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


It won't get as far as trying to start QEMU. The error message in the linked 
bug:
  error: unable to stat: /var/lib/libvirt/images/slic.dat: No such file or 
directory
comes from the DAC driver.

IIUC in virSecurityStackTransactionCommit we happily commit the SELinux
changes, fail to commit the DAC changes, but the rollback calling
virSecurityManagerTransactionAbort does nothing.

And since qemuSecuritySetAllLabel does not complete successfully, 
qemuProcessLaunch
does not ask its callers to restore the labels.

Jano


Signed-off-by: Michal Privoznik 
---
src/security/security_selinux.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)



signature.asc
Description: PGP signature

Re: [PATCH v3 0/5] Add support for two i386 pm options which control acpi hotplug

2021-09-20 Thread Ani Sinha

Ping

On Mon, Sep 13, 2021 at 1:39 PM Ani Sinha  wrote:

>
>
> On Sun, 12 Sep 2021, Ani Sinha wrote:
>
> > Hi all:
> >
> > This patchset introduces libvirt xml support for the following two pm
> conf
> > options:
> >
> > 
> >   
> >   
> > 
> >
>
> Another option is to create a new xml tag and add these under that tag.
> Something like:
>
> +   
> + 
> + 
> +   
>
> These are not exactly power management options. So putting them under 
> may not be correct.
> If this is a better approach I will resend the patchset with the changes
> along with addressing other review comments.
>
>
> > The above two options are only available for qemu driver and that too
> for x86
> > guests only. Both of them are global options.
> >
> > ``acpi-hotplug-bridge`` option enables or disables ACPI hotplug support
> for cold
> > plugged bridges. Examples of cold plugged bridges include PCI-PCI bridge
> > (pci-bridge controller) for pc machines and pcie-root-port controller
> for q35
> > machines. The corresponding commandline options to qemu for x86 guests
> are:
> >
> > (pc machines): -global
> PIIX4_PM.acpi-pci-hotplug-with-bridge-support=
> > (q35 machines): -global
> ICH9-LPC.acpi-pci-hotplug-with-bridge-support=
> >
> > Being global options, no other bridge specific options for pci-bridge
> > controller or pcie-root-port controllers are required. For pc machine
> type in
> > x86, this option is available in qemu for a long time, from version 2.1.
> > Please see the changes in qemu.git:
> >
> > 9e047b982452c6 ("piix4: add acpi pci hotplug support")
> > 133a2da488062e ("pc: acpi: generate AML only for PCI0 devices if PCI
> bridge hotplug is disabled")
> >
> > For q35 machine type, this was introduced in qemu 6.1 with the following
> > changes in qemu.git:
> >
> > (a) c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
> > (b) 17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on
> Q35")
> >
> > The reasons for enabling ACPI based hotplug for PCIe (q35) based
> machines (as
> > opposed to native hotplug) for bridges are outlined in (b). It is
> possible that
> > some users might still want to use native hotplug on PCIe [1]. Therefore,
> > this conf option enables users to choose either ACPI based hotplug or
> native
> > hotplug for cold plugged bridges (for example for pcie root port
> controller
> > in q35 machines).
> >
> > ``acpi-root-hotplug`` option enables or disables ACPI based hotplug for
> PCI root
> > bus (pci-root controller). This option is only available for pc machine
> type.
> > The corresponding commandline option to qemu for x86 guests is:
> >
> > -global PIIX4_PM.acpi-root-pci-hotplug=
> >
> > This additional option enables users to disable hotplug for all devices
> in the
> > system without adding an additional PCI-PCI bridge, putting the devices
> behind
> > the bridge and using the existing ``acpi-hotplug-bridge`` option to
> disable
> > hotplug on that bridge. This feature was introduced from qemu version
> 5.2 with
> > the following change in qemu.git:
> >
> > 3d7e78aaf ("Introduce a new flag for i440fx to disable PCI hotplug
> on the root bus")
> >
> > The above qemu commit describes some compelling reasons why users might
> to
> > disable hotplug on PCI root buses [2].
> >
> > A brief summary of the patches:
> >
> > >[PATCH v3 1/5] qemu: capablities: detect presence of
> > >[PATCH v3 2/5] qemu: capablities: detect presence of
> > Patches 1 and 2 implement support for qemu capability checks for the
> above
> > config options.
> >
> > >[PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and
> > Patch 3 actually adds the config option to the schema and adds related
> unit
> > tests.
> >
> > >[PATCH v3 4/5] qemu: command: add support for qemu options that
> > Patch 4 adds the backend qemu commandline support for the options. It
> also adds
> > relevant unit tests for the same.
> >
> > >[PATCH v3 5/5] NEWS: add new acpi pci hotplug options in the release
> > Patch 5 adds the release notes for the next libvirt release.
> >
> >
> > Changelog:
> > v1: initial implementation. Had some bugs and missed some unit tests.
> > v2: fixed bugs and added additional missing unit tests.
> > v3: reorganized the patches as per Laine's suggestion. Added more
> > details in commit messages. Added conf description in
> formatdomain.rst.
> > Added changelog for next release.
> >
> >
> > Notes:
> >
> > [1] One concrete example of why one might still want to use native
> hotplug with
> > pcie-root-port controller is the fact that we are still discovering
> issues with
> > acpi hotplug on PCIE. One such issue is:
> > https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02146.html
> > Another reason is that users have been using native hotplug on pcie root
> ports
> > up until now. They have built and tested their systems based on native
> hotplug.
> > They may not want to suddenly move to acpi based hotplug just because it
> is now
> > the default in qemu. Supporting the option to chose

[PATCH 08/14] qemu: capabilities: Retire QEMU_CAPS_SPLASH_TIMEOUT

2021-09-20 Thread Peter Krempa

The code assumes that the feature tracked by this capability always
exists.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c   | 3 +--
 src/qemu/qemu_capabilities.h   | 2 +-
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   | 1 -
 41 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0c6c8a7e9a..175b6bd239 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -308,7 +308,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "memory-backend-file", /* QEMU_CAPS_OBJECT_MEMORY_FILE */
   "usb-audio", /* QEMU_CAPS_OBJECT_USB_AUDIO */
   "rtc-reset-reinjection", /* QEMU_CAPS_RTC_RESET_REINJECTION */
-  "splash-timeout", /* QEMU_CAPS_SPLASH_TIMEOUT */
+  "splash-timeout", /* X_QEMU_CAPS_SPLASH_TIMEOUT */
   "iothread", /* QEMU_CAPS_OBJECT_IOTHREAD */

   /* 175 */
@@ -3203,7 +3203,6 @@ struct virQEMUCapsCommandLineProps {
  * in qemu and thus isn't being properly extended. Other means to detect
  * features should be used if possible. */
 static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
-{ "boot-opts", "splash-time", QEMU_CAPS_SPLASH_TIMEOUT },
 { "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE },
 { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
 { "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also 
checked fsdev->dmode */
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 70baf2810c..aa4c314e92 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -287,7 +287,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_OBJECT_MEMORY_FILE, /* -object memory-backend-file */
 QEMU_CAPS_OBJECT_USB_AUDIO, /* usb-audio device support */
 QEMU_CAPS_RTC_RESET_REINJECTION, /* rtc-reset-reinjection monitor command 
*/
-QEMU_CAPS_SPLASH_TIMEOUT, /* -boot splash-time */
+X_QEMU_CAPS_SPLASH_TIMEOUT, /* -boot splash-time */
 QEMU_CAPS_OBJECT_IOTHREAD, /* -object iothread */

 /* 175 */
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index d4095da183..fa770b65df 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -35,7 +35,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 57a5d1a585..93c2a6d1b6 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -75,7 +75,6 @@

[PATCH 14/14] qemu: capabilities: Retire QEMU_CAPS_FW_CFG

2021-09-20 Thread Peter Krempa

The code assumes that all supported qemu versions have this capability
so we can retire it.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c   | 3 +--
 src/qemu/qemu_capabilities.h   | 2 +-
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   | 1 -
 41 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ce0fff5d60..5e40cb86d6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -587,7 +587,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   /* 370 */
   "cpu.migratable", /* QEMU_CAPS_CPU_MIGRATABLE */
   "query-cpu-model-expansion.migratable", /* 
QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_MIGRATABLE */
-  "fw_cfg", /* QEMU_CAPS_FW_CFG */
+  "fw_cfg", /* X_QEMU_CAPS_FW_CFG */
   "migration-param.bandwidth", /* 
QEMU_CAPS_MIGRATION_PARAM_BANDWIDTH */
   "migration-param.downtime", /* 
QEMU_CAPS_MIGRATION_PARAM_DOWNTIME */

@@ -3206,7 +3206,6 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE },
 { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
 { "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also 
checked fsdev->dmode */
-{ "fw_cfg", "file", QEMU_CAPS_FW_CFG },
 { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP },
 { "numa", NULL, QEMU_CAPS_NUMA }, /* not needed after qemuCaps->version < 
300 */
 { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index dd7b6c6bef..27bbb2ea45 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -567,7 +567,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 /* 370 */
 QEMU_CAPS_CPU_MIGRATABLE, /* -cpu ...,migratable=on|off */
 QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_MIGRATABLE, /* 
query-cpu-model-expansion supports migratable:false */
-QEMU_CAPS_FW_CFG, /* -fw_cfg command line option */
+X_QEMU_CAPS_FW_CFG, /* -fw_cfg command line option */
 QEMU_CAPS_MIGRATION_PARAM_BANDWIDTH, /* max-bandwidth field in 
migrate-set-parameters */
 QEMU_CAPS_MIGRATION_PARAM_DOWNTIME, /* downtime-limit field in 
migrate-set-parameters */

diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 7ef7018228..bb68c1376a 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -98,7 +98,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml

[PATCH 13/14] qemu: Assume QEMU_CAPS_FW_CFG

2021-09-20 Thread Peter Krempa

qemu supports this since 81b2b81062 ("fw_cfg: insert fw_cfg file blobs
via qemu cmdline") released in qemu-v2.4.0 and it can't be compiled out.

Assume that the option always works and remove the corresponding check.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_validate.c | 12 ++--
 tests/qemuxml2argvtest.c |  2 +-
 tests/qemuxml2xmltest.c  |  2 +-
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index c4b7269589..c94a4b2eb3 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -886,18 +886,10 @@ qemuValidateDomainDefConsole(const virDomainDef *def,


 static int
-qemuValidateDomainDefSysinfo(const virSysinfoDef *def,
- virQEMUCaps *qemuCaps)
+qemuValidateDomainDefSysinfo(const virSysinfoDef *def)
 {
 size_t i;

-if (def->type == VIR_SYSINFO_FWCFG &&
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_FW_CFG)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("fw_cfg is not supported with this QEMU"));
-return -1;
-}
-
 for (i = 0; i < def->nfw_cfgs; i++) {
 const virSysinfoFWCfgDef *f = >fw_cfgs[i];

@@ -1239,7 +1231,7 @@ qemuValidateDomainDef(const virDomainDef *def,
 }

 for (i = 0; i < def->nsysinfo; i++) {
-if (qemuValidateDomainDefSysinfo(def->sysinfo[i], qemuCaps) < 0)
+if (qemuValidateDomainDefSysinfo(def->sysinfo[i]) < 0)
 return -1;
 }

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 77e384f67c..947f73e78e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1864,7 +1864,7 @@ mymain(void)
 DO_TEST_NOCAPS("smbios");
 DO_TEST_PARSE_ERROR_NOCAPS("smbios-date");
 DO_TEST_PARSE_ERROR_NOCAPS("smbios-uuid-match");
-DO_TEST("smbios-type-fwcfg", QEMU_CAPS_FW_CFG);
+DO_TEST_NOCAPS("smbios-type-fwcfg");

 DO_TEST_NOCAPS("watchdog");
 DO_TEST_NOCAPS("watchdog-device");
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 05cb87bbd1..147a00d03f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1107,7 +1107,7 @@ mymain(void)
 QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL);
 DO_TEST_NOCAPS("smbios");
 DO_TEST_NOCAPS("smbios-multiple-type2");
-DO_TEST("smbios-type-fwcfg", QEMU_CAPS_FW_CFG);
+DO_TEST_NOCAPS("smbios-type-fwcfg");

 DO_TEST_CAPS_LATEST("os-firmware-bios");
 DO_TEST_CAPS_LATEST("os-firmware-efi");
-- 
2.31.1

[PATCH 12/14] qemu: capabilities: Assume QEMU_CAPS_LOADPARM for s390 only

2021-09-20 Thread Peter Krempa

Added to 'query-command-line-options' in qemu commit 5559716c98
("util/qemu-config: Add loadparm to qemu machine_opts") released in
qemu-v2.10.0 but makes sense for s390 only. Treat it the same as the
keywrap capabilities in previous commit.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c   | 2 +-
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   | 1 -
 33 files changed, 1 insertion(+), 33 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 69a3f11fe4..ce0fff5d60 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3208,7 +3208,6 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also 
checked fsdev->dmode */
 { "fw_cfg", "file", QEMU_CAPS_FW_CFG },
 { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP },
-{ "machine", "loadparm", QEMU_CAPS_LOADPARM },
 { "numa", NULL, QEMU_CAPS_NUMA }, /* not needed after qemuCaps->version < 
300 */
 { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT },
 { "sandbox", "enable", QEMU_CAPS_SECCOMP_SANDBOX },
@@ -5002,6 +5001,7 @@ virQEMUCapsInitQMPBasicArch(virQEMUCaps *qemuCaps)
 case VIR_ARCH_S390X:
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_AES_KEY_WRAP);
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEA_KEY_WRAP);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_LOADPARM);
 break;

 case VIR_ARCH_ALPHA:
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 62326b50d7..11e4fbf06c 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -141,7 +141,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index 4ed9ffec2e..f41b8c334c 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -102,7 +102,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index 4fb441ca0a..0a0430cfdd 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -96,7 +96,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index 3e439f4634..a975fb268c 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -138,7 +138,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
index 9303420502..dbfbbcc2d0 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
@@ -95,7 +95,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml 
b/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml
index 6c976825d8..e2f2ebadc0 100644

[PATCH 11/14] qemu: capabilities: Assume QEMU_CAPS_AES_KEY_WRAP and QEMU_CAPS_DEA_KEY_WRAP for s390 only

2021-09-20 Thread Peter Krempa

qemu introduced these options in 2eb1cd0768 ("s390x: CPACF: Handle key
wrap machine options") released in qemu-v2.3.0 but was exposed in
query-command-line-options only in 5bcfa0c543 ("util/qemu-config: fix
missing machine command line options").

The problem is that they are exposed even for architectures which don't
actually in fact support those.

Make the two capabilities a bit more useful by assuming them only on
s390 and thus removing them from other arches.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c   | 10 ++
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |  2 --
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml|  2 --
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  |  2 --
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml|  2 --
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml|  2 --
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  |  2 --
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml|  2 --
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml|  2 --
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml|  2 --
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml|  2 --
 tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml  |  2 --
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   |  2 --
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   |  2 --
 33 files changed, 6 insertions(+), 68 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d26a3c97c1..69a3f11fe4 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3207,8 +3207,6 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
 { "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also 
checked fsdev->dmode */
 { "fw_cfg", "file", QEMU_CAPS_FW_CFG },
-{ "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP },
-{ "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP },
 { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP },
 { "machine", "loadparm", QEMU_CAPS_LOADPARM },
 { "numa", NULL, QEMU_CAPS_NUMA }, /* not needed after qemuCaps->version < 
300 */
@@ -5000,6 +4998,12 @@ virQEMUCapsInitQMPBasicArch(virQEMUCaps *qemuCaps)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
 break;

+case VIR_ARCH_S390:
+case VIR_ARCH_S390X:
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_AES_KEY_WRAP);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEA_KEY_WRAP);
+break;
+
 case VIR_ARCH_ALPHA:
 case VIR_ARCH_PPC:
 case VIR_ARCH_PPCEMB:
@@ -5007,8 +5011,6 @@ virQEMUCapsInitQMPBasicArch(virQEMUCaps *qemuCaps)
 case VIR_ARCH_SH4EB:
 case VIR_ARCH_RISCV32:
 case VIR_ARCH_RISCV64:
-case VIR_ARCH_S390:
-case VIR_ARCH_S390X:
 case VIR_ARCH_SPARC:
 case VIR_ARCH_SPARC64:
 case VIR_ARCH_ARMV6L:
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 0eefa61e1c..62326b50d7 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -82,8 +82,6 @@
   
   
   
-  
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index 8124a6c5fe..4ed9ffec2e 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -59,8 +59,6 @@
   
   
   
-  
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index 6873374877..4fb441ca0a 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -59,8 +59,6 @@
   
   
   
-

[PATCH 10/14] qemu: capabilities: QEMU_CAPS_MEM_MERGE

2021-09-20 Thread Peter Krempa

The code assumes that the feature tracked by this capability always
exists.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c   | 3 +--
 src/qemu/qemu_capabilities.h   | 2 +-
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   | 1 -
 41 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 175b6bd239..d26a3c97c1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -267,7 +267,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "vfio-pci.bootindex", /* X_QEMU_CAPS_VFIO_PCI_BOOTINDEX */
   "scsi-generic", /* X_QEMU_CAPS_DEVICE_SCSI_GENERIC */
   "scsi-generic.bootindex", /* 
X_QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX */
-  "mem-merge", /* QEMU_CAPS_MEM_MERGE */
+  "mem-merge", /* X_QEMU_CAPS_MEM_MERGE */

   /* 145 */
   "vnc-websocket", /* X_QEMU_CAPS_VNC_WEBSOCKET */
@@ -3207,7 +3207,6 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
 { "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also 
checked fsdev->dmode */
 { "fw_cfg", "file", QEMU_CAPS_FW_CFG },
-{ "machine", "mem-merge", QEMU_CAPS_MEM_MERGE },
 { "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP },
 { "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP },
 { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index aa4c314e92..dd7b6c6bef 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -246,7 +246,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 X_QEMU_CAPS_VFIO_PCI_BOOTINDEX, /* bootindex param for vfio-pci device */
 X_QEMU_CAPS_DEVICE_SCSI_GENERIC, /* -device scsi-generic */
 X_QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, /* -device 
scsi-generic.bootindex */
-QEMU_CAPS_MEM_MERGE, /* -machine mem-merge */
+X_QEMU_CAPS_MEM_MERGE, /* -machine mem-merge */

 /* 145 */
 X_QEMU_CAPS_VNC_WEBSOCKET, /* -vnc x:y,websocket */
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index fa770b65df..7ef7018228 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -26,7 +26,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 93c2a6d1b6..0eefa61e1c 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@

[PATCH 09/14] qemu: Always assume QEMU_CAPS_MEM_MERGE

2021-09-20 Thread Peter Krempa

Supported since qemu commit 8490fc78e7 ("add -machine mem-merge=on|off
option") released in qemu-v1.3.0 and can't be compiled out.

Assume that it's present and remove the validation code.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_validate.c | 7 ---
 tests/qemuxml2argvtest.c | 2 +-
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index eb985956e4..c4b7269589 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -757,13 +757,6 @@ qemuValidateDomainDefMemory(const virDomainDef *def,
 return -1;
 }

-if (mem->nosharepages && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_MERGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-_("disable shared memory is not available "
-  "with this QEMU binary"));
-return -1;
-}
-
 return 0;
 }

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1cbbf9bd24..77e384f67c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1291,7 +1291,7 @@ mymain(void)
 QEMU_CAPS_OBJECT_MEMORY_FILE);
 DO_TEST_CAPS_LATEST("hugepages-memaccess3");
 DO_TEST_CAPS_LATEST("hugepages-nvdimm");
-DO_TEST("nosharepages", QEMU_CAPS_MEM_MERGE);
+DO_TEST_NOCAPS("nosharepages");
 DO_TEST_NOCAPS("disk-cdrom");
 DO_TEST_CAPS_VER("disk-cdrom", "2.12.0");
 DO_TEST_CAPS_LATEST("disk-cdrom");
-- 
2.31.1

Re: [PATCH 8/8] qapi: add blockdev-replace command

2021-09-20 Thread Vladimir Sementsov-Ogievskiy


02.08.2021 21:54, Vladimir Sementsov-Ogievskiy wrote:

Add command that can add and remove filters.

Key points of functionality:

What the command does is simply replace some BdrvChild.bs by some other
nodes. The tricky thing is selecting there BdrvChild objects.
To be able to select any kind of BdrvChild we use a generic parent_id,
which may be a node-name, or qdev id or block export id. In future we
may support block jobs.

Any kind of ambiguity leads to error. If we have both device named
device0 and block export named device0 and they both point to same BDS,
user can't replace root child of one of these parents. So, to be able
to do replacements, user should avoid duplicating names in different
parent namespaces.

So, command allows to replace any single child in the graph.

On the other hand we want to realize a kind of bdrv_replace_node(),
which works well when we want to replace all parents of some node. For
this kind of task @parents-mode argument implemented.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/block-core.json | 78 +
  block.c  | 82 
  2 files changed, 160 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 675d8265eb..8059b96341 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5433,3 +5433,81 @@
  { 'command': 'blockdev-snapshot-delete-internal-sync',
'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
'returns': 'SnapshotInfo' }
+
+##
+# @BlockdevReplaceParentsMode:
+#
+# Alternative (to directly set @parent) way to chose parents in
+# @blockdev-replace
+#
+# @exactly-one: Exactly one parent should match a condition, otherwise
+#   @blockdev-replace fails.
+#
+# @all: All matching parents are taken into account. If replacing lead
+#   to loops in block graph, @blockdev-replace fails.
+#
+# @auto: Same as @all, but automatically skip replacing parents if it
+#leads to loops in block graph.
+#
+# Since: 6.2
+##
+{ 'enum': 'BlockdevReplaceParentsMode',
+  'data': ['exactly-one', 'all', 'auto'] }
+
+##
+# @BlockdevReplace:
+#
+# Declaration of one replacement.
+#
+# @parent: id of parent. It may be qdev or block export or simple
+#  node-name. If id is ambiguous (for example node-name of
+#  some BDS equals to block export name), blockdev-replace
+#  fails. If not specified, blockdev-replace goes through
+#  @parents-mode scenario, see below. Note, that @parent and
+#  @parents-mode can't be specified simultaneously.
+#  If @parent is specified, only one edge is selected. If
+#  several edges match the condition, blockdev-replace fails.
+#
+# @edge: name of the child. If omitted, any child name matches.
+#
+# @child: node-name of the child. If omitted, any child matches.
+# Must be present if @parent is not specified.
+#
+# @parents-mode: declares how to select edge (or edges) when @parent
+#is omitted. Default is 'one'.
+#
+# Since: 6.2
+#
+# Examples:
+#
+# 1. Change root node of some device.
+#
+# Note, that @edge name is omitted, as
+# devices always has only one child. As well, no need in specifying
+# old @child.
+#
+# -> { "parent": "device0", "new-child": "some-node-name" }
+#
+# 2. Insert copy-before-write filter.
+#
+# Assume, after blockdev-add we have block-node 'source', with several
+# writing parents and one copy-before-write 'filter' parent. And we want
+# to actually insert the filter. We do:
+#
+# -> { "child": "source", "parent-mode": "auto", "new-child": "filter" }
+#
+# All parents of source would be switched to 'filter' node, except for
+# 'filter' node itself (otherwise, it will make a loop in block-graph).
+##
+{ 'struct': 'BlockdevReplace',
+  'data': { '*parent': 'str', '*edge': 'str', '*child': 'str',
+'*parents-mode': 'BlockdevReplaceParentsMode',
+'new-child': 'str' } }



We also can make 'new-child' a 'BlockdevRef' and allow creating and inserting 
the filter in one command.


--
Best regards,
Vladimir

[PATCH 05/14] qemu: capabilities: Retire QEMU_CAPS_REBOOT_TIMEOUT

2021-09-20 Thread Peter Krempa

The code assumes that the feature tracked by this capability always
exists.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c   | 3 +--
 src/qemu/qemu_capabilities.h   | 2 +-
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   | 1 -
 41 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b67dd07fe1..0c6c8a7e9a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -217,7 +217,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "ide-drive.wwn", /* QEMU_CAPS_IDE_DRIVE_WWN */
   "scsi-disk.wwn", /* QEMU_CAPS_SCSI_DISK_WWN */
   "seccomp-sandbox", /* QEMU_CAPS_SECCOMP_SANDBOX */
-  "reboot-timeout", /* QEMU_CAPS_REBOOT_TIMEOUT */
+  "reboot-timeout", /* X_QEMU_CAPS_REBOOT_TIMEOUT */
   "dump-guest-core", /* X_QEMU_CAPS_DUMP_GUEST_CORE */

   /* 110 */
@@ -3203,7 +3203,6 @@ struct virQEMUCapsCommandLineProps {
  * in qemu and thus isn't being properly extended. Other means to detect
  * features should be used if possible. */
 static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
-{ "boot-opts", "reboot-timeout", QEMU_CAPS_REBOOT_TIMEOUT },
 { "boot-opts", "splash-time", QEMU_CAPS_SPLASH_TIMEOUT },
 { "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE },
 { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 103ae7db01..70baf2810c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -196,7 +196,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_IDE_DRIVE_WWN, /* Is ide-drive.wwn available? */
 QEMU_CAPS_SCSI_DISK_WWN, /* Is scsi-disk.wwn available? */
 QEMU_CAPS_SECCOMP_SANDBOX, /* -sandbox */
-QEMU_CAPS_REBOOT_TIMEOUT, /* -boot reboot-timeout */
+X_QEMU_CAPS_REBOOT_TIMEOUT, /* -boot reboot-timeout */
 X_QEMU_CAPS_DUMP_GUEST_CORE, /* dump-guest-core-parameter */

 /* 110 */
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index cda8de1ae7..d4095da183 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -17,7 +17,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 9112b5791e..57a5d1a585 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -39,7 +39,6 @@
   
   
   
-  
   
   
   
diff --git

[PATCH 04/14] qemu: Always assume QEMU_CAPS_REBOOT_TIMEOUT

2021-09-20 Thread Peter Krempa

Supported since ac05f34924 ("add a boot parameter to set reboot
timeout") released in qemu-v1.3.0 and can't be compiled out.

Assume that it's present and remove the validation code.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_validate.c | 9 -
 tests/qemuxml2argvtest.c | 4 ++--
 tests/qemuxml2xmltest.c  | 4 ++--
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 6b685881a8..4eadcc6aae 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -598,15 +598,6 @@ qemuValidateDomainDefBoot(const virDomainDef *def,
 }
 }

-if (def->os.bios.rt_set) {
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("reboot timeout is not supported "
- "by this QEMU binary"));
-return -1;
-}
-}
-
 if (def->os.bm_timeout_set) {
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 64408caeff..e13aeb4b15 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1174,8 +1174,8 @@ mymain(void)
 driver.config->nogfxAllowHostAudio = false;
 g_unsetenv("QEMU_AUDIO_DRV");

-DO_TEST("reboot-timeout-disabled", QEMU_CAPS_REBOOT_TIMEOUT);
-DO_TEST("reboot-timeout-enabled", QEMU_CAPS_REBOOT_TIMEOUT);
+DO_TEST_NOCAPS("reboot-timeout-disabled");
+DO_TEST_NOCAPS("reboot-timeout-enabled");

 DO_TEST("bios",
 QEMU_CAPS_DEVICE_ISA_SERIAL);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 6d3526f91f..8329f871b3 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -233,8 +233,8 @@ mymain(void)
 DO_TEST_NOCAPS("boot-menu-disable-with-timeout");
 DO_TEST_NOCAPS("boot-order");

-DO_TEST("reboot-timeout-enabled", QEMU_CAPS_REBOOT_TIMEOUT);
-DO_TEST("reboot-timeout-disabled", QEMU_CAPS_REBOOT_TIMEOUT);
+DO_TEST_NOCAPS("reboot-timeout-enabled");
+DO_TEST_NOCAPS("reboot-timeout-disabled");

 DO_TEST_NOCAPS("clock-utc");
 DO_TEST_NOCAPS("clock-localtime");
-- 
2.31.1

[PATCH 02/14] qemu: capabilities: Retire QEMU_CAPS_BOOT_STRICT

2021-09-20 Thread Peter Krempa

It's not used since last commit.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c   | 3 +--
 src/qemu/qemu_capabilities.h   | 2 +-
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml  | 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml| 1 -
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   | 1 -
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   | 1 -
 41 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f27a621f8c..b67dd07fe1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -287,7 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "virtio-mmio", /* QEMU_CAPS_DEVICE_VIRTIO_MMIO */
   "ich9-intel-hda", /* QEMU_CAPS_DEVICE_ICH9_INTEL_HDA */
   "kvm-pit-lost-tick-policy", /* QEMU_CAPS_KVM_PIT_TICK_POLICY */
-  "boot-strict", /* QEMU_CAPS_BOOT_STRICT */
+  "boot-strict", /* X_QEMU_CAPS_BOOT_STRICT */
   "pvpanic", /* QEMU_CAPS_DEVICE_PANIC */

   /* 160 */
@@ -3203,7 +3203,6 @@ struct virQEMUCapsCommandLineProps {
  * in qemu and thus isn't being properly extended. Other means to detect
  * features should be used if possible. */
 static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
-{ "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT },
 { "boot-opts", "reboot-timeout", QEMU_CAPS_REBOOT_TIMEOUT },
 { "boot-opts", "splash-time", QEMU_CAPS_SPLASH_TIMEOUT },
 { "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS_COMMANDLINE },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f3379f556c..103ae7db01 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -266,7 +266,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DEVICE_VIRTIO_MMIO, /* -device virtio-mmio */
 QEMU_CAPS_DEVICE_ICH9_INTEL_HDA, /* -device ich9-intel-hda */
 QEMU_CAPS_KVM_PIT_TICK_POLICY, /* kvm-pit.lost_tick_policy */
-QEMU_CAPS_BOOT_STRICT, /* -boot strict */
+X_QEMU_CAPS_BOOT_STRICT, /* -boot strict */
 QEMU_CAPS_DEVICE_PANIC, /* -device pvpanic */

 /* 160 */
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 32c25f9e99..cda8de1ae7 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -30,7 +30,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 631f644144..9112b5791e 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -65,7 +65,6 @@
   
   
   
-  
   
   
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml

[PATCH 06/14] qemuxml2argvtest: Remove negative case for 'boot-menu-enable-with-timeout'

2021-09-20 Thread Peter Krempa

The feature is now always present. Remove the negative test case as the
upcomming commit will remove the checks.

Signed-off-by: Peter Krempa 
---
 tests/qemuxml2argvdata/boot-menu-enable-with-timeout.err | 1 -
 tests/qemuxml2argvtest.c | 1 -
 2 files changed, 2 deletions(-)
 delete mode 100644 tests/qemuxml2argvdata/boot-menu-enable-with-timeout.err

diff --git a/tests/qemuxml2argvdata/boot-menu-enable-with-timeout.err 
b/tests/qemuxml2argvdata/boot-menu-enable-with-timeout.err
deleted file mode 100644
index 3ac2abf11b..00
--- a/tests/qemuxml2argvdata/boot-menu-enable-with-timeout.err
+++ /dev/null
@@ -1 +0,0 @@
-unsupported configuration: splash timeout is not supported by this QEMU binary
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e13aeb4b15..e68b63c67d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1070,7 +1070,6 @@ mymain(void)
 DO_TEST_NOCAPS("boot-menu-enable");
 DO_TEST("boot-menu-enable-with-timeout",
 QEMU_CAPS_SPLASH_TIMEOUT);
-DO_TEST_PARSE_ERROR_NOCAPS("boot-menu-enable-with-timeout");
 DO_TEST_PARSE_ERROR("boot-menu-enable-with-timeout-invalid",
 QEMU_CAPS_SPLASH_TIMEOUT);
 DO_TEST_NOCAPS("boot-menu-disable");
-- 
2.31.1

[PATCH 07/14] qemu: Always assume QEMU_CAPS_SPLASH_TIMEOUT

2021-09-20 Thread Peter Krempa

Supported since qemu commit 3d3b8303c6 ("showing a splash picture when
start") released in qemu-v1.0 and can't be compiled out.

Assume that it's present and remove the validation code.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_validate.c | 14 ++
 tests/qemuxml2argvtest.c |  6 ++
 tests/qemuxml2xmltest.c  |  2 +-
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 4eadcc6aae..eb985956e4 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -566,8 +566,7 @@ qemuValidateDomainDefPM(const virDomainDef *def,


 static int
-qemuValidateDomainDefBoot(const virDomainDef *def,
-  virQEMUCaps *qemuCaps)
+qemuValidateDomainDefBoot(const virDomainDef *def)
 {
 if (def->os.loader &&
 def->os.loader->secure == VIR_TRISTATE_BOOL_YES) {
@@ -598,15 +597,6 @@ qemuValidateDomainDefBoot(const virDomainDef *def,
 }
 }

-if (def->os.bm_timeout_set) {
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("splash timeout is not supported "
- "by this QEMU binary"));
-return -1;
-}
-}
-
 return 0;
 }

@@ -1218,7 +1208,7 @@ qemuValidateDomainDef(const virDomainDef *def,
 if (qemuValidateDomainDefPM(def, qemuCaps) < 0)
 return -1;

-if (qemuValidateDomainDefBoot(def, qemuCaps) < 0)
+if (qemuValidateDomainDefBoot(def) < 0)
 return -1;

 if (qemuValidateDomainVCpuTopology(def, qemuCaps) < 0)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e68b63c67d..1cbbf9bd24 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1068,10 +1068,8 @@ mymain(void)
 QEMU_CAPS_ICH9_AHCI);
 DO_TEST_NOCAPS("boot-multi");
 DO_TEST_NOCAPS("boot-menu-enable");
-DO_TEST("boot-menu-enable-with-timeout",
-QEMU_CAPS_SPLASH_TIMEOUT);
-DO_TEST_PARSE_ERROR("boot-menu-enable-with-timeout-invalid",
-QEMU_CAPS_SPLASH_TIMEOUT);
+DO_TEST_NOCAPS("boot-menu-enable-with-timeout");
+DO_TEST_PARSE_ERROR_NOCAPS("boot-menu-enable-with-timeout-invalid");
 DO_TEST_NOCAPS("boot-menu-disable");
 DO_TEST_NOCAPS("boot-menu-disable-drive");
 DO_TEST_PARSE_ERROR("boot-dev+order",
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 8329f871b3..05cb87bbd1 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -228,7 +228,7 @@ mymain(void)
 QEMU_CAPS_DEVICE_IOH3420,
 QEMU_CAPS_ICH9_AHCI);
 DO_TEST_NOCAPS("boot-multi");
-DO_TEST("boot-menu-enable-with-timeout", QEMU_CAPS_SPLASH_TIMEOUT);
+DO_TEST_NOCAPS("boot-menu-enable-with-timeout");
 DO_TEST_NOCAPS("boot-menu-disable");
 DO_TEST_NOCAPS("boot-menu-disable-with-timeout");
 DO_TEST_NOCAPS("boot-order");
-- 
2.31.1

[PATCH 03/14] tests: qemuxml2argv: Remove negative case for 'reboot-timeout-enabled'

2021-09-20 Thread Peter Krempa

All supported qemu versions now support this feature so this test is
pointless.

Signed-off-by: Peter Krempa 
---
 tests/qemuxml2argvdata/reboot-timeout-enabled.err | 1 -
 tests/qemuxml2argvtest.c  | 1 -
 2 files changed, 2 deletions(-)
 delete mode 100644 tests/qemuxml2argvdata/reboot-timeout-enabled.err

diff --git a/tests/qemuxml2argvdata/reboot-timeout-enabled.err 
b/tests/qemuxml2argvdata/reboot-timeout-enabled.err
deleted file mode 100644
index 317b293168..00
--- a/tests/qemuxml2argvdata/reboot-timeout-enabled.err
+++ /dev/null
@@ -1 +0,0 @@
-unsupported configuration: reboot timeout is not supported by this QEMU binary
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 74a9f5a0ee..64408caeff 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1176,7 +1176,6 @@ mymain(void)

 DO_TEST("reboot-timeout-disabled", QEMU_CAPS_REBOOT_TIMEOUT);
 DO_TEST("reboot-timeout-enabled", QEMU_CAPS_REBOOT_TIMEOUT);
-DO_TEST_PARSE_ERROR_NOCAPS("reboot-timeout-enabled");

 DO_TEST("bios",
 QEMU_CAPS_DEVICE_ISA_SERIAL);
-- 
2.31.1

[PATCH 00/14] qemu: more 'query-command-line-options' cleanups

2021-09-20 Thread Peter Krempa

In a private conversation Markus dug out the history of certain of the
flags we probe via 'query-command-line-options'. I already had some
patches for this but without the history or justification.

To prevent us going through this again I've decided to send some more
removal of capability bits based on stuff that all qemu's we support
have.

Peter Krempa (14):
  qemu: command: Always assume 'QEMU_CAPS_BOOT_STRICT'
  qemu: capabilities: Retire QEMU_CAPS_BOOT_STRICT
  tests: qemuxml2argv: Remove negative case for 'reboot-timeout-enabled'
  qemu: Always assume QEMU_CAPS_REBOOT_TIMEOUT
  qemu: capabilities: Retire QEMU_CAPS_REBOOT_TIMEOUT
  qemuxml2argvtest: Remove negative case for
'boot-menu-enable-with-timeout'
  qemu: Always assume QEMU_CAPS_SPLASH_TIMEOUT
  qemu: capabilities: Retire QEMU_CAPS_SPLASH_TIMEOUT
  qemu: Always assume QEMU_CAPS_MEM_MERGE
  qemu: capabilities:  QEMU_CAPS_MEM_MERGE
  qemu: capabilities: Assume QEMU_CAPS_AES_KEY_WRAP and
QEMU_CAPS_DEA_KEY_WRAP for s390 only
  qemu: capabilities: Assume QEMU_CAPS_LOADPARM for s390 only
  qemu: Assume QEMU_CAPS_FW_CFG
  qemu: capabilities: Retire QEMU_CAPS_FW_CFG

 src/qemu/qemu_capabilities.c  | 27 ++--
 src/qemu/qemu_capabilities.h  | 10 ++---
 src/qemu/qemu_command.c   | 10 ++---
 src/qemu/qemu_validate.c  | 42 ++-
 .../caps_2.11.0.s390x.xml |  5 ---
 .../caps_2.11.0.x86_64.xml|  8 
 .../caps_2.12.0.aarch64.xml   |  8 
 .../caps_2.12.0.ppc64.xml |  8 
 .../caps_2.12.0.s390x.xml |  5 ---
 .../caps_2.12.0.x86_64.xml|  8 
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  8 
 .../caps_3.0.0.riscv32.xml|  8 
 .../caps_3.0.0.riscv64.xml|  8 
 .../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  5 ---
 .../caps_3.0.0.x86_64.xml |  8 
 .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  8 
 .../caps_3.1.0.x86_64.xml |  8 
 .../caps_4.0.0.aarch64.xml|  8 
 .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |  8 
 .../caps_4.0.0.riscv32.xml|  8 
 .../caps_4.0.0.riscv64.xml|  8 
 .../qemucapabilitiesdata/caps_4.0.0.s390x.xml |  5 ---
 .../caps_4.0.0.x86_64.xml |  8 
 .../caps_4.1.0.x86_64.xml |  8 
 .../caps_4.2.0.aarch64.xml|  8 
 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |  8 
 .../qemucapabilitiesdata/caps_4.2.0.s390x.xml |  5 ---
 .../caps_4.2.0.x86_64.xml |  8 
 .../caps_5.0.0.aarch64.xml|  8 
 .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |  8 
 .../caps_5.0.0.riscv64.xml|  8 
 .../caps_5.0.0.x86_64.xml |  8 
 .../qemucapabilitiesdata/caps_5.1.0.sparc.xml |  8 
 .../caps_5.1.0.x86_64.xml |  8 
 .../caps_5.2.0.aarch64.xml|  8 
 .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml |  8 
 .../caps_5.2.0.riscv64.xml|  8 
 .../qemucapabilitiesdata/caps_5.2.0.s390x.xml |  5 ---
 .../caps_5.2.0.x86_64.xml |  8 
 .../caps_6.0.0.aarch64.xml|  8 
 .../qemucapabilitiesdata/caps_6.0.0.s390x.xml |  5 ---
 .../caps_6.0.0.x86_64.xml |  8 
 .../caps_6.1.0.x86_64.xml |  8 
 .../aarch64-aavmf-virtio-mmio.args|  1 +
 tests/qemuxml2argvdata/aarch64-acpi-uefi.args |  1 +
 .../aarch64-cpu-passthrough.args  |  1 +
 tests/qemuxml2argvdata/aarch64-gic-host.args  |  1 +
 .../aarch64-gic-none-tcg.args |  1 +
 tests/qemuxml2argvdata/aarch64-gic-v2.args|  1 +
 tests/qemuxml2argvdata/aarch64-gic-v3.args|  1 +
 .../aarch64-kvm-32-on-64.args |  1 +
 .../aarch64-noacpi-nouefi.args|  1 +
 .../qemuxml2argvdata/aarch64-noacpi-uefi.args |  1 +
 .../qemuxml2argvdata/aarch64-pci-serial.args  |  1 +
 .../aarch64-traditional-pci.args  |  1 +
 .../aarch64-usb-controller-nec-xhci.args  |  1 +
 .../aarch64-usb-controller-qemu-xhci.args |  1 +
 .../aarch64-video-default.args|  1 +
 .../aarch64-video-virtio-gpu-pci.args |  1 +
 .../aarch64-virt-2.6-virtio-pci-default.args  |  1 +
 .../aarch64-virt-default-nic.args |  1 +
 .../qemuxml2argvdata/aarch64-virt-virtio.args |  1 +
 .../aarch64-virtio-pci-default.args   |  1 +
 .../aarch64-virtio-pci-manual-addresses.args  |  1 +
 tests/qemuxml2argvdata/acpi-table.args|  1 +
 .../arm-vexpressa9-basic.args |  1 +
 .../arm-vexpressa9-nodevs.args|  1 +
 .../arm-vexpressa9-virtio.args|  1 +

Re: [PATCH 8/8] qapi: add blockdev-replace command

2021-09-20 Thread Vladimir Sementsov-Ogievskiy


Thanks a lot for reviewing!

20.09.2021 09:44, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


Add command that can add and remove filters.

Key points of functionality:

What the command does is simply replace some BdrvChild.bs by some other
nodes. The tricky thing is selecting there BdrvChild objects.
To be able to select any kind of BdrvChild we use a generic parent_id,
which may be a node-name, or qdev id or block export id. In future we
may support block jobs.

Any kind of ambiguity leads to error. If we have both device named
device0 and block export named device0 and they both point to same BDS,
user can't replace root child of one of these parents. So, to be able
to do replacements, user should avoid duplicating names in different
parent namespaces.

So, command allows to replace any single child in the graph.

On the other hand we want to realize a kind of bdrv_replace_node(),
which works well when we want to replace all parents of some node. For
this kind of task @parents-mode argument implemented.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/block-core.json | 78 +
  block.c  | 82 
  2 files changed, 160 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 675d8265eb..8059b96341 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5433,3 +5433,81 @@
  { 'command': 'blockdev-snapshot-delete-internal-sync',
'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
'returns': 'SnapshotInfo' }
+
+##
+# @BlockdevReplaceParentsMode:
+#
+# Alternative (to directly set @parent) way to chose parents in
+# @blockdev-replace
+#
+# @exactly-one: Exactly one parent should match a condition, otherwise
+#   @blockdev-replace fails.
+#
+# @all: All matching parents are taken into account. If replacing lead
+#   to loops in block graph, @blockdev-replace fails.
+#
+# @auto: Same as @all, but automatically skip replacing parents if it
+#leads to loops in block graph.
+#
+# Since: 6.2
+##
+{ 'enum': 'BlockdevReplaceParentsMode',
+  'data': ['exactly-one', 'all', 'auto'] }
+
+##
+# @BlockdevReplace:
+#
+# Declaration of one replacement.


Replacement of what?  A node in the block graph?


A specific child node in one or in several edges




+#
+# @parent: id of parent. It may be qdev or block export or simple
+#  node-name.


It may also be a QOM path, because find_device_state() interprets
arguments starting with '/' as QOM paths.

When is a node name "simple"?

Suggest: It may be a qdev ID, a QOM path, a block export ID, or a node
name.


OK



The trouble is of course that we're merging three separate name spaces.


Yes. Alternatively we can also add an enum of node-type [bds, device, export[, 
job]], and select graph nodes more explicit (by pair of id/path/name and type)

But if we have to use these things in one context, it seems good to enforce 
users use different names for them. And in this way, we can avoid strict typing.



Aside: a single name space for IDs would be so much saner, but we
screwed that up long ago.


If id is ambiguous (for example node-name of
+#  some BDS equals to block export name), blockdev-replace
+#  fails.


Is there a way out of this situation, or are is replacement simply
impossible then?


In my idea, it's simply impossible. If someone want to use this new interface, 
he should care to use different names for different things.




If not specified, blockdev-replace goes through
+#  @parents-mode scenario, see below. Note, that @parent and
+#  @parents-mode can't be specified simultaneously.


What if neither is specified?  Hmm, @parents-mode has a default, so
that's what we get.


+#  If @parent is specified, only one edge is selected. If
+#  several edges match the condition, blockdev-replace fails.
+#
+# @edge: name of the child. If omitted, any child name matches.
+#
+# @child: node-name of the child. If omitted, any child matches.
+# Must be present if @parent is not specified.


Is @child useful when @parent is present?


You may specify @child and @parent, to replace child in specific edge. Or 
@parent and @edge. Or all three fields: just to be strict.



What's the difference between "name of the child" and "node name of the
child"?


Although we have to deal with different kinds of nodes (BDS, exports, blks, 
...),
children are always BDS.

But, may be in the context, it's better say "id of the child".




+#
+# @parents-mode: declares how to select edge (or edges) when @parent
+#is omitted. Default is 'one'.


'exactly-one'

Minor combinatorial explosion.  There are four optional arguments, one
of them an enum, and only some combination of argument presence and enum
value are valid.  For a serious review, I'd have to make a table of
combinations, then

[PATCH 4/4] qemuMonitorJSONGetStatus: Refactor cleanup

2021-09-20 Thread Peter Krempa

Use g_autofree for the JSON values to remove cleanup label and ret
variable.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_monitor_json.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8d3c4031a6..37e9c05d27 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1710,10 +1710,9 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon,
  bool *running,
  virDomainPausedReason *reason)
 {
-int ret = -1;
 const char *status;
-virJSONValue *cmd;
-virJSONValue *reply = NULL;
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
 virJSONValue *data;

 if (reason)
@@ -1723,17 +1722,17 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon,
 return -1;

 if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
+return -1;

 if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
-goto cleanup;
+return -1;

 data = virJSONValueObjectGetObject(reply, "return");

 if (virJSONValueObjectGetBoolean(data, "running", running) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("query-status reply was missing running state"));
-goto cleanup;
+return -1;
 }

 if ((status = virJSONValueObjectGetString(data, "status"))) {
@@ -1743,12 +1742,7 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon,
 VIR_DEBUG("query-status reply was missing status details");
 }

-ret = 0;
-
- cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
-return ret;
+return 0;
 }


-- 
2.31.1

[PATCH 3/4] util: virstring: Remove unused 'virStringListMerge'

2021-09-20 Thread Peter Krempa

Signed-off-by: Peter Krempa 
---
 src/libvirt_private.syms |  1 -
 src/util/virstring.c | 35 ---
 src/util/virstring.h |  3 ---
 3 files changed, 39 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ace35d709f..25ee21463c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3298,7 +3298,6 @@ virStringHasControlChars;
 virStringHasSuffix;
 virStringIsEmpty;
 virStringIsPrintable;
-virStringListMerge;
 virStringMatch;
 virStringMatchesNameSuffix;
 virStringParsePort;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index f416fed3c5..cee56debca 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -35,41 +35,6 @@

 VIR_LOG_INIT("util.string");

-/**
- * virStringListMerge:
- * @dst: a NULL-terminated array of strings to expand
- * @src: a NULL-terminated array of strings
- *
- * Merges @src into @dst. Upon successful return from this
- * function, @dst is resized to $(dst + src) elements and @src is
- * freed.
- *
- * Returns 0 on success, -1 otherwise.
- */
-int
-virStringListMerge(char ***dst,
-   char ***src)
-{
-size_t dst_len, src_len, i;
-
-if (!src || !*src)
-return 0;
-
-dst_len = g_strv_length(*dst);
-src_len = g_strv_length(*src);
-
-VIR_REALLOC_N(*dst, dst_len + src_len + 1);
-
-for (i = 0; i <= src_len; i++)
-(*dst)[i + dst_len] = (*src)[i];
-
-/* Don't call g_strfreev() as it would free strings in
- * @src. */
-VIR_FREE(*src);
-return 0;
-}
-
-
 /* Like strtol, but produce an "int" result, and check more carefully.
Return 0 upon success;  return -1 to indicate failure.
When END_PTR is NULL, the byte after the final valid digit must be NUL.
diff --git a/src/util/virstring.h b/src/util/virstring.h
index ca81889c9b..45f07ddd7a 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -22,9 +22,6 @@

 #define VIR_INT64_STR_BUFLEN 21

-int virStringListMerge(char ***dst,
-   char ***src);
-
 int virStrToLong_i(char const *s,
char **end_ptr,
int base,
-- 
2.31.1

[PATCH 1/4] util: virdevmapper: Sanitize use of macros for buffer size

2021-09-20 Thread Peter Krempa

There are two distinct uses of an arbitrary buffers size when querying
the device mapper. One is related to loading the /proc/devices file,
while the other is used as buffer for ioctls to the devmapper.

Split up the macros used here so that it's clear that they are not meant
for the same thing.

Signed-off-by: Peter Krempa 
---
 src/util/virdevmapper.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 2c4c2df999..301c8f3ba7 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -42,12 +42,13 @@
 VIR_LOG_INIT("util.virdevmapper");

 # define PROC_DEVICES "/proc/devices"
+# define PROC_DEVICES_BUF_SIZE (16 * 1024)
 # define DM_NAME "device-mapper"
 # define DEV_DM_DIR "/dev/" DM_DIR
 # define CONTROL_PATH DEV_DM_DIR "/" DM_CONTROL_NODE
-# define BUF_SIZE (16 * 1024)

-G_STATIC_ASSERT(BUF_SIZE > sizeof(struct dm_ioctl));
+# define VIR_DEVMAPPER_IOCTL_BUF_SIZE_INCREMENT (16 * 1024)
+G_STATIC_ASSERT(VIR_DEVMAPPER_IOCTL_BUF_SIZE_INCREMENT > sizeof(struct 
dm_ioctl));


 static int
@@ -60,7 +61,7 @@ virDevMapperGetMajor(unsigned int *major)
 if (!virFileExists(CONTROL_PATH))
 return -2;

-if (virFileReadAll(PROC_DEVICES, BUF_SIZE, ) < 0)
+if (virFileReadAll(PROC_DEVICES, PROC_DEVICES_BUF_SIZE, ) < 0)
 return -1;

 lines = g_strsplit(buf, "\n", 0);
@@ -92,7 +93,7 @@ virDevMapperGetMajor(unsigned int *major)
 static void *
 virDMIoctl(int controlFD, int cmd, struct dm_ioctl *dm, char **buf)
 {
-size_t bufsize = BUF_SIZE;
+size_t bufsize = VIR_DEVMAPPER_IOCTL_BUF_SIZE_INCREMENT;

  reread:
 *buf = g_new0(char, bufsize);
@@ -113,7 +114,7 @@ virDMIoctl(int controlFD, int cmd, struct dm_ioctl *dm, 
char **buf)
 memcpy(dm, *buf, sizeof(struct dm_ioctl));

 if (dm->flags & DM_BUFFER_FULL_FLAG) {
-bufsize += BUF_SIZE;
+bufsize += VIR_DEVMAPPER_IOCTL_BUF_SIZE_INCREMENT;
 VIR_FREE(*buf);
 goto reread;
 }
-- 
2.31.1

[PATCH 0/4] Various cleanups

2021-09-20 Thread Peter Krempa

Few patches from old branches.

Peter Krempa (4):
  util: virdevmapper: Sanitize use of macros for buffer size
  virDevMapperGetTargets: Use a linked list as return type
  util: virstring: Remove unused 'virStringListMerge'
  qemuMonitorJSONGetStatus: Refactor cleanup

 src/libvirt_private.syms |  1 -
 src/qemu/qemu_cgroup.c   | 11 
 src/qemu/qemu_monitor_json.c | 18 +
 src/qemu/qemu_namespace.c|  9 +++
 src/util/virdevmapper.c  | 49 +---
 src/util/virdevmapper.h  |  2 +-
 src/util/virstring.c | 35 --
 src/util/virstring.h |  3 ---
 tests/qemuhotplugmock.c  | 10 +++-
 9 files changed, 43 insertions(+), 95 deletions(-)

-- 
2.31.1

[PATCH 2/4] virDevMapperGetTargets: Use a linked list as return type

2021-09-20 Thread Peter Krempa

Of the two callers one simply iterates over the returned paths and the
second one appends the returned paths to another linked list. Simplify
all of this by directly returning a linked list.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_cgroup.c| 11 ++-
 src/qemu/qemu_namespace.c |  9 +++--
 src/util/virdevmapper.c   | 38 +-
 src/util/virdevmapper.h   |  2 +-
 tests/qemuhotplugmock.c   | 10 --
 5 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 6d4a82b3cd..471cbc3b8f 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -38,6 +38,7 @@
 #include "virnuma.h"
 #include "virdevmapper.h"
 #include "virutil.h"
+#include "virglibutil.h"

 #define VIR_FROM_THIS VIR_FROM_QEMU

@@ -60,8 +61,8 @@ qemuSetupImagePathCgroup(virDomainObj *vm,
 {
 qemuDomainObjPrivate *priv = vm->privateData;
 int perms = VIR_CGROUP_DEVICE_READ;
-g_auto(GStrv) targetPaths = NULL;
-size_t i;
+g_autoptr(virGSListString) targetPaths = NULL;
+GSList *n;
 int rv;

 if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
@@ -94,10 +95,10 @@ qemuSetupImagePathCgroup(virDomainObj *vm,
 return -1;
 }

-for (i = 0; targetPaths && targetPaths[i]; i++) {
-rv = virCgroupAllowDevicePath(priv->cgroup, targetPaths[i], perms, 
false);
+for (n = targetPaths; n; n = n->next) {
+rv = virCgroupAllowDevicePath(priv->cgroup, n->data, perms, false);

-virDomainAuditCgroupPath(vm, priv->cgroup, "allow", targetPaths[i],
+virDomainAuditCgroupPath(vm, priv->cgroup, "allow", n->data,
  virCgroupGetDevicePermsString(perms),
  rv);
 if (rv < 0)
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 728d77fc61..f1aaca86b1 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -251,8 +251,7 @@ qemuDomainSetupDisk(virStorageSource *src,
 if (!(tmpPath = 
virPCIDeviceAddressGetIOMMUGroupDev(>nvme->pciAddr)))
 return -1;
 } else {
-g_auto(GStrv) targetPaths = NULL;
-GStrv tmp;
+GSList *targetPaths;

 if (virStorageSourceIsEmpty(next) ||
 !virStorageSourceIsLocalStorage(next)) {
@@ -270,10 +269,8 @@ qemuDomainSetupDisk(virStorageSource *src,
 return -1;
 }

-if (targetPaths) {
-for (tmp = targetPaths; *tmp; tmp++)
-*paths = g_slist_prepend(*paths, g_steal_pointer(tmp));
-}
+if (targetPaths)
+*paths = g_slist_concat(g_slist_reverse(targetPaths), *paths);
 }

 *paths = g_slist_prepend(*paths, g_steal_pointer());
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 301c8f3ba7..e42324fd19 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -36,6 +36,7 @@
 # include "virstring.h"
 # include "virfile.h"
 # include "virlog.h"
+# include "virglibutil.h"

 # define VIR_FROM_THIS VIR_FROM_STORAGE

@@ -217,18 +218,16 @@ virDMSanitizepath(const char *path)
 static int
 virDevMapperGetTargetsImpl(int controlFD,
const char *path,
-   char ***devPaths_ret,
+   GSList **devPaths,
unsigned int ttl)
 {
 g_autofree char *sanitizedPath = NULL;
 g_autofree char *buf = NULL;
 struct dm_ioctl dm;
 struct dm_target_deps *deps = NULL;
-g_auto(GStrv) devPaths = NULL;
 size_t i;

 memset(, 0, sizeof(dm));
-*devPaths_ret = NULL;

 if (ttl == 0) {
 errno = ELOOP;
@@ -258,24 +257,17 @@ virDevMapperGetTargetsImpl(int controlFD,
 return -1;
 }

-devPaths = g_new0(char *, deps->count + 1);
 for (i = 0; i < deps->count; i++) {
-devPaths[i] = g_strdup_printf("/dev/block/%u:%u",
-  major(deps->dev[i]),
-  minor(deps->dev[i]));
-}
-
-for (i = 0; i < deps->count; i++) {
-g_auto(GStrv) tmpPaths = NULL;
+char *curpath = g_strdup_printf("/dev/block/%u:%u",
+major(deps->dev[i]),
+minor(deps->dev[i]));

-if (virDevMapperGetTargetsImpl(controlFD, devPaths[i], , ttl 
- 1) < 0)
-return -1;
+*devPaths = g_slist_prepend(*devPaths, curpath);

-if (virStringListMerge(, ) < 0)
+if (virDevMapperGetTargetsImpl(controlFD, curpath, devPaths, ttl - 1) 
< 0)
 return -1;
 }

-*devPaths_ret = g_steal_pointer();
 return 0;
 }

@@ -283,11 +275,10 @@ virDevMapperGetTargetsImpl(int controlFD,
 /**
  * virDevMapperGetTargets:
  * @path: devmapper target
- * @devPaths: returned string list of devices
+ * @devPaths:

[RFC PATCH 7/7] testQEMUSchemaValidateEnum: Validate deprecated members

2021-09-20 Thread Peter Krempa

Starting from QEMU-6.2 enum members can be deprecated. Add support to
the validator.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 38dd0e14bc..b4b5eb1ed6 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -376,6 +376,12 @@ testQEMUSchemaValidateEnum(virJSONValue *obj,
 virJSONValue *member = virJSONValueArrayGet(members, i);

 if (STREQ_NULLABLE(objstr, virJSONValueObjectGetString(member, 
"name"))) {
+int rc;
+
+/* the new 'members' array allows us to check deprecations */
+if ((rc = testQEMUSchemaValidateDeprecated(member, objstr, 
ctxt)) < 0)
+return rc;
+
 virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr));
 return 0;
 }
-- 
2.31.1

[RFC PATCH 6/7] testQEMUSchemaValidateDeprecated: Move to the top

2021-09-20 Thread Peter Krempa

Move the function to the top of the file so other functions placed
towards the top will be able to reuse it.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 82 ++---
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 7398c73ccc..38dd0e14bc 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -28,6 +28,47 @@ struct testQEMUSchemaValidateCtxt {
 };


+static int
+testQEMUSchemaValidateDeprecated(virJSONValue *root,
+ const char *name,
+ struct testQEMUSchemaValidateCtxt *ctxt)
+{
+virJSONValue *features = virJSONValueObjectGetArray(root, "features");
+size_t nfeatures;
+size_t i;
+
+if (!features)
+return 0;
+
+nfeatures = virJSONValueArraySize(features);
+
+for (i = 0; i < nfeatures; i++) {
+virJSONValue *cur = virJSONValueArrayGet(features, i);
+const char *curstr;
+
+if (!cur ||
+!(curstr = virJSONValueGetString(cur))) {
+virBufferAsprintf(ctxt->debug, "ERROR: features of '%s' are 
malformed", name);
+return -2;
+}
+
+if (STREQ(curstr, "deprecated")) {
+if (ctxt->allowDeprecated) {
+virBufferAsprintf(ctxt->debug, "WARNING: '%s' is deprecated", 
name);
+if (virTestGetVerbose())
+g_fprintf(stderr, "\nWARNING: '%s' is deprecated\n", name);
+return 0;
+} else {
+virBufferAsprintf(ctxt->debug, "ERROR: '%s' is deprecated", 
name);
+return -1;
+}
+}
+}
+
+return 0;
+}
+
+
 static int
 testQEMUSchemaValidateRecurse(virJSONValue *obj,
   virJSONValue *root,
@@ -461,47 +502,6 @@ testQEMUSchemaValidateAlternate(virJSONValue *obj,
 }


-static int
-testQEMUSchemaValidateDeprecated(virJSONValue *root,
- const char *name,
- struct testQEMUSchemaValidateCtxt *ctxt)
-{
-virJSONValue *features = virJSONValueObjectGetArray(root, "features");
-size_t nfeatures;
-size_t i;
-
-if (!features)
-return 0;
-
-nfeatures = virJSONValueArraySize(features);
-
-for (i = 0; i < nfeatures; i++) {
-virJSONValue *cur = virJSONValueArrayGet(features, i);
-const char *curstr;
-
-if (!cur ||
-!(curstr = virJSONValueGetString(cur))) {
-virBufferAsprintf(ctxt->debug, "ERROR: features of '%s' are 
malformed", name);
-return -2;
-}
-
-if (STREQ(curstr, "deprecated")) {
-if (ctxt->allowDeprecated) {
-virBufferAsprintf(ctxt->debug, "WARNING: '%s' is deprecated", 
name);
-if (virTestGetVerbose())
-g_fprintf(stderr, "\nWARNING: '%s' is deprecated\n", name);
-return 0;
-} else {
-virBufferAsprintf(ctxt->debug, "ERROR: '%s' is deprecated", 
name);
-return -1;
-}
-}
-}
-
-return 0;
-}
-
-
 static int
 testQEMUSchemaValidateRecurse(virJSONValue *obj,
   virJSONValue *root,
-- 
2.31.1

[RFC PATCH 5/7] testQEMUSchemaValidateEnum: Use new 'members' for 'enum' meta type

2021-09-20 Thread Peter Krempa

Switch to the new more featured way to report enum members which will
also allow us to detect use of deprecated members.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index e75345b582..7398c73ccc 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -319,6 +319,7 @@ testQEMUSchemaValidateEnum(virJSONValue *obj,
 {
 const char *objstr;
 virJSONValue *values = NULL;
+virJSONValue *members = NULL;
 size_t i;

 if (virJSONValueGetType(obj) != VIR_JSON_TYPE_STRING) {
@@ -328,6 +329,22 @@ testQEMUSchemaValidateEnum(virJSONValue *obj,

 objstr = virJSONValueGetString(obj);

+/* qemu-6.2 added a "members" array superseding "values" */
+if ((members = virJSONValueObjectGetArray(root, "members"))) {
+for (i = 0; i < virJSONValueArraySize(members); i++) {
+virJSONValue *member = virJSONValueArrayGet(members, i);
+
+if (STREQ_NULLABLE(objstr, virJSONValueObjectGetString(member, 
"name"))) {
+virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr));
+return 0;
+}
+}
+
+virBufferAsprintf(ctxt->debug, "ERROR: enum value '%s' is not in 
schema",
+  NULLSTR(objstr));
+return -1;
+}
+
 if ((values = virJSONValueObjectGetArray(root, "values"))) {
 for (i = 0; i < virJSONValueArraySize(values); i++) {
 virJSONValue *value = virJSONValueArrayGet(values, i);
-- 
2.31.1

[RFC PATCH 1/7] virQEMUQAPISchemaTraverseEnum: Move helper variables into loop

2021-09-20 Thread Peter Krempa

Move them closer to where they are actually used.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_qapi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c
index 36b184b226..165ecf1180 100644
--- a/src/qemu/qemu_qapi.c
+++ b/src/qemu/qemu_qapi.c
@@ -243,8 +243,6 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur,
 {
 const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt);
 virJSONValue *values;
-virJSONValue *enumval;
-const char *value;
 size_t i;

 if (query[0] != '^')
@@ -259,6 +257,9 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur,
 return -2;

 for (i = 0; i < virJSONValueArraySize(values); i++) {
+virJSONValue *enumval;
+const char *value;
+
 if (!(enumval = virJSONValueArrayGet(values, i)) ||
 !(value = virJSONValueGetString(enumval)))
 continue;
-- 
2.31.1

[RFC PATCH 3/7] virQEMUQAPISchemaTraverseEnum: Allow query of enume type features

2021-09-20 Thread Peter Krempa

QEMU-6.2 added feature flags for enum types. Add support for querying
them into our QMP schema query language.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_qapi.c | 26 ++
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c
index 790f7c0fee..426db8d30d 100644
--- a/src/qemu/qemu_qapi.c
+++ b/src/qemu/qemu_qapi.c
@@ -242,6 +242,7 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur,
   struct virQEMUQAPISchemaTraverseContext *ctxt)
 {
 const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt);
+const char *featurequery = NULL;
 virJSONValue *values;
 virJSONValue *members;
 size_t i;
@@ -249,8 +250,16 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur,
 if (query[0] != '^')
 return 0;

-if (virQEMUQAPISchemaTraverseContextHasNextQuery(ctxt))
-return -3;
+if (virQEMUQAPISchemaTraverseContextHasNextQuery(ctxt)) {
+/* we might have a query for a feature flag of an enum value */
+featurequery = virQEMUQAPISchemaTraverseContextNextQuery(ctxt);
+
+if (*featurequery != '$' ||
+virQEMUQAPISchemaTraverseContextHasNextQuery(ctxt))
+return -3;
+
+featurequery++;
+}

 query++;

@@ -263,13 +272,21 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur,
 if (!member || !(name = virJSONValueObjectGetString(member, 
"name")))
 return -2;

-if (STREQ(name, query))
+if (STREQ(name, query)) {
+if (featurequery)
+return 
virQEMUQAPISchemaTraverseHasObjectFeature(featurequery, member);
+
 return 1;
+}
 }

 return 0;
 }

+/* old-style "values" array doesn't have feature flags so any query is 
necessarily false */
+if (featurequery)
+return 0;
+
 if (!(values = virJSONValueObjectGetArray(cur, "values")))
 return -2;

@@ -439,7 +456,8 @@ virQEMUQAPISchemaTraverse(const char *baseName,
  *
  * The above types can be chained arbitrarily using slashes to construct any
  * path into the schema tree, booleans must be always the last component as 
they
- * don't refer to a type.
+ * don't refer to a type. An exception is querying feature of an enum value
+ * (.../^enumval/$featurename) which is allowed.
  *
  * Returns 1 if @query was found in @schema filling @entry if non-NULL, 0 if
  * @query was not found in @schema and -1 on other errors along with an 
appropriate
-- 
2.31.1

[RFC PATCH 4/7] testQEMUSchemaValidateEnum: Refactor logic to simplify switching to new QMP schema format

2021-09-20 Thread Peter Krempa

QEMU-6.2 is reporting enum values in the new 'members' array which we'll
be switching to. Rewrite the logic so that adding the new checker is
more straightforward.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 29 ++---
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index f6347231a8..e75345b582 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -319,7 +319,6 @@ testQEMUSchemaValidateEnum(virJSONValue *obj,
 {
 const char *objstr;
 virJSONValue *values = NULL;
-virJSONValue *value;
 size_t i;

 if (virJSONValueGetType(obj) != VIR_JSON_TYPE_STRING) {
@@ -329,24 +328,24 @@ testQEMUSchemaValidateEnum(virJSONValue *obj,

 objstr = virJSONValueGetString(obj);

-if (!(values = virJSONValueObjectGetArray(root, "values"))) {
-virBufferAsprintf(ctxt->debug, "ERROR: missing enum values in schema 
'%s'",
-  NULLSTR(virJSONValueObjectGetString(root, "name")));
-return -2;
-}
-
-for (i = 0; i < virJSONValueArraySize(values); i++) {
-value = virJSONValueArrayGet(values, i);
+if ((values = virJSONValueObjectGetArray(root, "values"))) {
+for (i = 0; i < virJSONValueArraySize(values); i++) {
+virJSONValue *value = virJSONValueArrayGet(values, i);

-if (STREQ_NULLABLE(objstr, virJSONValueGetString(value))) {
-virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr));
-return 0;
+if (STREQ_NULLABLE(objstr, virJSONValueGetString(value))) {
+virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr));
+return 0;
+}
 }
+
+virBufferAsprintf(ctxt->debug, "ERROR: enum value '%s' is not in 
schema",
+  NULLSTR(objstr));
+return -1;
 }

-virBufferAsprintf(ctxt->debug, "ERROR: enum value '%s' is not in schema",
-  NULLSTR(objstr));
-return -1;
+virBufferAsprintf(ctxt->debug, "ERROR: missing enum values in schema '%s'",
+  NULLSTR(virJSONValueObjectGetString(root, "name")));
+return -2;
 }


-- 
2.31.1

[RFC PATCH 2/7] virQEMUQAPISchemaTraverseEnum: Use the modern 'members' array

2021-09-20 Thread Peter Krempa

Starting from QEMU-6.2 enum members are reported as an array of objects
under new name "values" so that extra data can be reported for each
member.

Modify the code so that we prefer 'members' and skip 'values' completely
if we've used 'members'.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_qapi.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c
index 165ecf1180..790f7c0fee 100644
--- a/src/qemu/qemu_qapi.c
+++ b/src/qemu/qemu_qapi.c
@@ -243,6 +243,7 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur,
 {
 const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt);
 virJSONValue *values;
+virJSONValue *members;
 size_t i;

 if (query[0] != '^')
@@ -253,6 +254,22 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur,

 query++;

+/* qemu-6.2 added a "members" array superseding "values" */
+if ((members = virJSONValueObjectGetArray(cur, "members"))) {
+for (i = 0; i < virJSONValueArraySize(members); i++) {
+virJSONValue *member = virJSONValueArrayGet(members, i);
+const char *name;
+
+if (!member || !(name = virJSONValueObjectGetString(member, 
"name")))
+return -2;
+
+if (STREQ(name, query))
+return 1;
+}
+
+return 0;
+}
+
 if (!(values = virJSONValueObjectGetArray(cur, "values")))
 return -2;

-- 
2.31.1

[RFC PATCH 0/7] qemu: QAPI-schema: Add support for enum value 'features'

2021-09-20 Thread Peter Krempa

This series is based on Markus' effort to add 'feature' flags to enum
values so that e.g. they can be deprecated:

https://listman.redhat.com/archives/libvir-list/2021-September/msg00453.html

This series adapts both the schema query language to support querying
for arbitrary feature flags and also the schema validator to reject
deprecated enum values.

Libvirt didn't use any deprecated value for now so to see that this
really works you can fetch this series including patches which show it's
working (tests fail):

 git fetch https://gitlab.com/pipo.sk/libvirt.git qemu-deprecate-enum

The above branch also demonstrates that parsing from the new arrays
produces identical results as it has updated qemu capabilities.

This series is RFC as it's waiting for the qemu additions first.

Peter Krempa (7):
  virQEMUQAPISchemaTraverseEnum: Move helper variables into loop
  virQEMUQAPISchemaTraverseEnum: Use the modern 'members' array
  virQEMUQAPISchemaTraverseEnum: Allow query of enume type features
  testQEMUSchemaValidateEnum: Refactor logic to simplify switching to
new QMP schema format
  testQEMUSchemaValidateEnum: Use new 'members' for 'enum' meta type
  testQEMUSchemaValidateDeprecated: Move to the top
  testQEMUSchemaValidateEnum: Validate deprecated members

 src/qemu/qemu_qapi.c|  46 +++--
 tests/testutilsqemuschema.c | 130 +---
 2 files changed, 117 insertions(+), 59 deletions(-)

-- 
2.31.1

Re: [PATCH 13/14] virsh: Introduce virshCompleteEmpty and use it for places where we can't suggest anything

2021-09-20 Thread Ján Tomko


On a Thursday in 2021, Peter Krempa wrote:

For now this serves just as an annotation because readline and also the
bash completion script insist on completing local paths when an empty
list is returned.

This will serve for future reference once we'll be able to properly
refuse to suggest anything.

The completer is used for fields such as names for new objects,
description strings, password strings etc, URIs and hostnames which we
can't feasibly autocomplete.



My bash autocompletes hostnames from ssh's known_hosts. But I agree
that it might not be feasible.

Passwods, on the other hand, are trivially completable:
https://listman.redhat.com/archives/libvir-list/2019-April/msg00018.html

Jano


Signed-off-by: Peter Krempa 
---
tools/virsh-checkpoint.c |  2 ++
tools/virsh-completer.c  | 18 ++
tools/virsh-completer.h  |  5 +
tools/virsh-domain.c | 33 +
tools/virsh-pool.c   |  7 +++
tools/virsh-secret.c |  1 +
tools/virsh-snapshot.c   |  2 ++
tools/virsh-volume.c |  5 +
tools/virsh.c|  1 +
9 files changed, 74 insertions(+)


signature.asc
Description: PGP signature

Re: [PATCH 0/2] virsh: Fix fallback code path for vcpuinfo

2021-09-20 Thread Ján Tomko


On a Wednesday in 2021, Peter Krempa wrote:

Peter Krempa (2):
 virshDomainGetVcpuBitmap: Return bitmap when taking the fallback path
 virshDomainGetVcpuBitmap: Refactor cleanup

tools/virsh-domain.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH] gitlab: remove obsolete job rules for TEMPORARILY_DISABLED variable

2021-09-20 Thread Jiri Denemark

On Mon, Sep 20, 2021 at 13:41:07 +0100, Daniel P. Berrangé wrote:
> We previously had a 'rules:' entry that caused a job to be skipped if
> the variable "TEMPORARILY_DISABLED" was set. This is no longer needed
> since we can set a similar flag in ci/manifest.yml and re-generate
> to temporarily skip a job.
> 
> Unfortunately the 'rules:' entry had an unexpected side-effect on
> the pipelines that was never previously noticed. Instead of only
> running pipelines on push, the mere existance of the 'rules:' entry
> caused triggering of pipelines on merge requests too.
> 
> The newly auto-generated ci/gitlab.yml file does not have a 'rules:'
> for the container job template, and thus only runs on git push.
> 
> The result is that build jobs try to run on merge requests and the
> container jobs they depend on don't exist. This breaks the entire
> pipeline with a message that the config is invalid due to broken
> job dependencies.
> 
> This fixes a regression introduced in
> 
>   commit ccc7a44adbea003d6a0dc2f156adb2856c28bd4c
>   Author: Daniel P. Berrangé 
>   Date:   Thu Sep 9 14:49:01 2021 +0100
> 
> ci: re-generate containers/gitlab config from manifest
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  .gitlab-ci.yml | 8 
>  1 file changed, 8 deletions(-)

Reviewed-by: Jiri Denemark

[libvirt PATCH] gitlab: remove obsolete job rules for TEMPORARILY_DISABLED variable

2021-09-20 Thread Daniel P . Berrangé

We previously had a 'rules:' entry that caused a job to be skipped if
the variable "TEMPORARILY_DISABLED" was set. This is no longer needed
since we can set a similar flag in ci/manifest.yml and re-generate
to temporarily skip a job.

Unfortunately the 'rules:' entry had an unexpected side-effect on
the pipelines that was never previously noticed. Instead of only
running pipelines on push, the mere existance of the 'rules:' entry
caused triggering of pipelines on merge requests too.

The newly auto-generated ci/gitlab.yml file does not have a 'rules:'
for the container job template, and thus only runs on git push.

The result is that build jobs try to run on merge requests and the
container jobs they depend on don't exist. This breaks the entire
pipeline with a message that the config is invalid due to broken
job dependencies.

This fixes a regression introduced in

  commit ccc7a44adbea003d6a0dc2f156adb2856c28bd4c
  Author: Daniel P. Berrangé 
  Date:   Thu Sep 9 14:49:01 2021 +0100

ci: re-generate containers/gitlab config from manifest

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.yml | 8 
 1 file changed, 8 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b396a1511d..d486faca58 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -18,10 +18,6 @@ include: '/ci/gitlab.yml'
 
 .native_build_job:
   extends: .gitlab_native_build_job
-  rules:
-- if: "$TEMPORARILY_DISABLED"
-  allow_failure: true
-- when: on_success
   cache:
 paths:
   - ccache/
@@ -45,10 +41,6 @@ include: '/ci/gitlab.yml'
 paths:
   - ccache/
 key: "$CI_JOB_NAME"
-  rules:
-- if: "$TEMPORARILY_DISABLED"
-  allow_failure: true
-- when: on_success
   before_script:
 - *script_variables
   script:
-- 
2.31.1

[PATCH 1/2] selinux: Swap two blocks handling setfilecon_raw() failure

2021-09-20 Thread Michal Privoznik

In virSecuritySELinuxSetFileconImpl() we have code that handles
setfilecon_raw() failure. The code consists of two blocks: one
for dealing with shared filesystem like NFS (errno is ENOTSUP or
EROFS) and the other block that's dealing with EPERM for
privileged daemon. Well, the order of these two blocks is a bit
confusing because the comment above them mentions the NFS case
but EPERM block follows. Swap these two blocks to make it less
confusing.

Signed-off-by: Michal Privoznik 
---
 src/security/security_selinux.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 0e5ea0366d..050acee2b0 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1264,22 +1264,9 @@ virSecuritySELinuxSetFileconImpl(const char *path,
  * boolean tunables to allow it ...
  */
 VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
-if (setfilecon_errno != EOPNOTSUPP && setfilecon_errno != ENOTSUP &&
-setfilecon_errno != EROFS) {
+if (setfilecon_errno == EOPNOTSUPP || setfilecon_errno == ENOTSUP ||
+setfilecon_errno == EROFS) {
 VIR_WARNINGS_RESET
-/* However, don't claim error if SELinux is in Enforcing mode and
- * we are running as unprivileged user and we really did see EPERM.
- * Otherwise we want to return error if SELinux is Enforcing. */
-if (security_getenforce() == 1 &&
-(setfilecon_errno != EPERM || privileged)) {
-virReportSystemError(setfilecon_errno,
- _("unable to set security context '%s' on 
'%s'"),
- tcon, path);
-return -1;
-}
-VIR_WARN("unable to set security context '%s' on '%s' (errno %d)",
- tcon, path, setfilecon_errno);
-} else {
 const char *msg;
 if (virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 1 &&
 security_get_boolean_active("virt_use_nfs") != 1) {
@@ -1293,6 +1280,19 @@ virSecuritySELinuxSetFileconImpl(const char *path,
 VIR_INFO("Setting security context '%s' on '%s' not supported",
  tcon, path);
 }
+} else {
+/* However, don't claim error if SELinux is in Enforcing mode and
+ * we are running as unprivileged user and we really did see EPERM.
+ * Otherwise we want to return error if SELinux is Enforcing. */
+if (security_getenforce() == 1 &&
+(setfilecon_errno != EPERM || privileged)) {
+virReportSystemError(setfilecon_errno,
+ _("unable to set security context '%s' on 
'%s'"),
+ tcon, path);
+return -1;
+}
+VIR_WARN("unable to set security context '%s' on '%s' (errno %d)",
+ tcon, path, setfilecon_errno);
 }
 
 return 1;
-- 
2.32.0

[PATCH 2/2] selinux: Don't ignore ENOENT in Permissive mode

2021-09-20 Thread Michal Privoznik

In selinux driver there's virSecuritySELinuxSetFileconImpl()
which is responsible for actual setting of SELinux label on given
file and handling possible failures. In fhe failure handling code
we decide whether failure is fatal or not. But there is a bug:
depending on SELinux mode (Permissive vs. Enforcing) the ENOENT
is either ignored or considered fatal. This not correct - ENOENT
must always be fatal - QEMU will fail opening it anyways.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2004850
Signed-off-by: Michal Privoznik 
---
 src/security/security_selinux.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 050acee2b0..7e8c4fb4f2 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1283,9 +1283,11 @@ virSecuritySELinuxSetFileconImpl(const char *path,
 } else {
 /* However, don't claim error if SELinux is in Enforcing mode and
  * we are running as unprivileged user and we really did see EPERM.
- * Otherwise we want to return error if SELinux is Enforcing. */
-if (security_getenforce() == 1 &&
-(setfilecon_errno != EPERM || privileged)) {
+ * Otherwise we want to return error if SELinux is Enforcing, or we
+ * saw EPERM regardless of SELinux mode. */
+if (setfilecon_errno == ENOENT ||
+(security_getenforce() == 1 &&
+ (setfilecon_errno != EPERM || privileged))) {
 virReportSystemError(setfilecon_errno,
  _("unable to set security context '%s' on 
'%s'"),
  tcon, path);
-- 
2.32.0

[PATCH 0/2] selinux: Don't ignore ENOENT in Permissive mode

2021-09-20 Thread Michal Privoznik

See 2/2 for explanation.

Michal Prívozník (2):
  selinux: Swap two blocks handling setfilecon_raw() failure
  selinux: Don't ignore ENOENT in Permissive mode

 src/security/security_selinux.c | 32 +---
 1 file changed, 17 insertions(+), 15 deletions(-)

-- 
2.32.0

Re: [PATCH RFC 1/5] qapi: Enable enum member introspection to show more than name

2021-09-20 Thread Peter Krempa

On Mon, Sep 20, 2021 at 11:08:59 +0200, Markus Armbruster wrote:
> Peter Krempa  writes:
> 
> > On Wed, Sep 15, 2021 at 21:24:21 +0200, Markus Armbruster wrote:
> >> The next commit will add feature flags to enum members.  There's a
> >> problem, though: query-qmp-schema shows an enum type's members as an
> >> array of member names (SchemaInfoEnum member @values).  If it showed
> >> an array of objects with a name member, we could simply add more
> >> members to these objects.  Since it's just strings, we can't.
> >> 
> >> I can see three ways to correct this design mistake:
> >> 
> >> 1. Do it the way we should have done it, plus compatibility goo.
> >> 
> >>We want a ['SchemaInfoEnumMember'] member in SchemaInfoEnum.  Since
> >>changing @values would be a compatibility break, add a new member
> >>@members instead.
> >> 
> >>@values is now redundant.  We should be able to get rid of it
> >>eventually.
> >> 
> >>In my testing, output of qemu-system-x86_64's query-qmp-schema
> >>grows by 11% (18.5KiB).
> >
> > I prefer this one. While the schema output grows, nobody is really
> > reading it manually.
> 
> True, but growing schema output can only slow down client startup.
> Negligible for libvirt, I presume?

Libvirt employs caching, so unless it's the first VM started after a
qemu/libvirt upgrade, the results are already processed and cached.

In fact we don't even keep the full schema around, we just extract
information and store them as capability bits. For now we didn't run
into the need to have the full schema around when starting a VM.

[...]

> >> 3. Versioned query-qmp-schema.
> >> 
> >>query-qmp-schema provides either @values or @members.  The QMP
> >>client can select which version it wants.
> >
> > At least for libvirt this poses a chicken & egg problem. We'd have to
> > query the schema to see that it has the switch to do the selection and
> > then probe with the modern one.
> 
> The simplest solution is to try the versions the management application
> can understand in order of preference (newest to oldest) until one
> succeeds.  I'd expect the first try to work most of the time.  Only when
> you combine new libvirt with old QEMU, the fallback has to kick in.
> 
> Other parts of the management application should remain oblivous of the
> differences.

That would certainly work and be reasonably straightforward for libvirt
to implement, but:
 1) libvirt's code for using the QMP schema would be exactly the same as
with approach 1), as we need to handle old clients too and the new
way is simply a superset of what we have
 2) qemu's deprecation approach itself wouldn't be any easier in either
of those scenarios

Basically the only thing this would gain us is that if the deprecation
period is over old clients which were not fixed could fail silently:

Assuming that 'query-qmp-schema' gains a boolean option such as
'fancier-enums' and setting that to true returns the new format of
schema, after the deprecation is over you could simply return an error
if a caller omits 'fancier-enums' or sets it to false, which creates a
clean cut for the removal.

With approach 1) itself, clients which were not adapted would start
lacking information based on enum values.

Now for those it depends on how they actually handled it until now. E.g.
old libvirt would report that the QMP schema is broken if 'values' would
be missing.

Whether that's a worthwhile thing to do? I'm not really persuaded. (And
I'm biased since libvirt handles it correctly).

> We could of course try to reduce the number of roundtrips, say by
> putting sufficient information into the QMP greeting (one roundtrip), or
> the output of query-qmp-schema (try oldest to find the best one, then
> try the best one unless it's the oldest).  I doubt that's worthwhile.

In this particular scenario, I'd doubt that it's worthwhile as the
change isn't really fundamental and issuing one extra QMP call isn't as
problematic as other cases, e.g probing of CPU features which results in
a QMP call per feature when starting a VM.

Currently libvirt issues 50 + 5 QMP commands(kvm, and non-kvm) for
probing capabilities.

> I'm not trying to talk you into this solution.  We're just exploring the
> solution space together, and with an open mind.

The idea of unconditionally trying a newer approach is a good one to
hold onto when we'll need it in the future!

Re: [PATCH RFC 1/5] qapi: Enable enum member introspection to show more than name

2021-09-20 Thread Markus Armbruster

Peter Krempa  writes:

> On Wed, Sep 15, 2021 at 21:24:21 +0200, Markus Armbruster wrote:
>> The next commit will add feature flags to enum members.  There's a
>> problem, though: query-qmp-schema shows an enum type's members as an
>> array of member names (SchemaInfoEnum member @values).  If it showed
>> an array of objects with a name member, we could simply add more
>> members to these objects.  Since it's just strings, we can't.
>> 
>> I can see three ways to correct this design mistake:
>> 
>> 1. Do it the way we should have done it, plus compatibility goo.
>> 
>>We want a ['SchemaInfoEnumMember'] member in SchemaInfoEnum.  Since
>>changing @values would be a compatibility break, add a new member
>>@members instead.
>> 
>>@values is now redundant.  We should be able to get rid of it
>>eventually.
>> 
>>In my testing, output of qemu-system-x86_64's query-qmp-schema
>>grows by 11% (18.5KiB).
>
> I prefer this one. While the schema output grows, nobody is really
> reading it manually.

True, but growing schema output can only slow down client startup.
Negligible for libvirt, I presume?

> The implementation in libvirt is very straightforward:
>
> https://gitlab.com/pipo.sk/libvirt/-/commit/24f1709cfae72cd765d02dc2124d6c954554ea55
> https://gitlab.com/pipo.sk/libvirt/-/commit/5909db9d4994327b3f64d5c329edd4b175495bfe
>
>> 2. Like 1, but omit "boring" elements of @member, and empty @member.
>> 
>>@values does not become redundant.  Output of query-qmp-schema
>>grows only as we make enum members non-boring.
>
> This has 2 drawbacks:
> - we would never get rid of either of those
> - clients would have to check both, one for whether the member is
>   present and second whether it's non-boring.
>
> IMO it's simpler for clients just to prefer the new approach when
> present as the old is simply a subset.

Noted.

>> 3. Versioned query-qmp-schema.
>> 
>>query-qmp-schema provides either @values or @members.  The QMP
>>client can select which version it wants.
>
> At least for libvirt this poses a chicken & egg problem. We'd have to
> query the schema to see that it has the switch to do the selection and
> then probe with the modern one.

The simplest solution is to try the versions the management application
can understand in order of preference (newest to oldest) until one
succeeds.  I'd expect the first try to work most of the time.  Only when
you combine new libvirt with old QEMU, the fallback has to kick in.

Other parts of the management application should remain oblivous of the
differences.

We could of course try to reduce the number of roundtrips, say by
putting sufficient information into the QMP greeting (one roundtrip), or
the output of query-qmp-schema (try oldest to find the best one, then
try the best one unless it's the oldest).  I doubt that's worthwhile.

I'm not trying to talk you into this solution.  We're just exploring the
solution space together, and with an open mind.

Re: [PATCH RFC 1/5] qapi: Enable enum member introspection to show more than name

2021-09-20 Thread Markus Armbruster

Eric Blake  writes:

> On Wed, Sep 15, 2021 at 09:24:21PM +0200, Markus Armbruster wrote:
>> The next commit will add feature flags to enum members.  There's a
>> problem, though: query-qmp-schema shows an enum type's members as an
>> array of member names (SchemaInfoEnum member @values).  If it showed
>> an array of objects with a name member, we could simply add more
>> members to these objects.  Since it's just strings, we can't.
>> 
>> I can see three ways to correct this design mistake:
>> 
>> 1. Do it the way we should have done it, plus compatibility goo.
>> 
>>We want a ['SchemaInfoEnumMember'] member in SchemaInfoEnum.  Since
>>changing @values would be a compatibility break, add a new member
>>@members instead.
>> 
>>@values is now redundant.  We should be able to get rid of it
>>eventually.
>> 
>>In my testing, output of qemu-system-x86_64's query-qmp-schema
>>grows by 11% (18.5KiB).
>
> This makes sense if we plan to deprecate @values - if so, that
> deprecation would make sense as part of this series, although we may
> drag our feet for how long before we actually remove it.

Yes.  Changing query-qmp-schema requires extra care, as it is the very
means for coping with change.

>> 
>> 2. Like 1, but omit "boring" elements of @member, and empty @member.
>> 
>>@values does not become redundant.  Output of query-qmp-schema
>>grows only as we make enum members non-boring.
>
> Does not change whether libvirt would have to learn to query the new
> members, but adds a mandatory fallback step for learning about boring
> members (although the fallback step will have to be there anyway for
> older qemu).  Peter probably has a better idea of which version is
> nicer.
>
>> 
>> 3. Versioned query-qmp-schema.
>> 
>>query-qmp-schema provides either @values or @members.  The QMP
>>client can select which version it wants.
>
> Sounds more complicated to implement.  I'm not opposed to it, but am
> leaning towards 1 or 2 myself.

More on this in my reply to Peter.

>
>> 
>> This commit implements 1. simply because it's the solution I thought
>> of first.  I'm prepared to implement one of the others if we decide
>> that's what we want.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  qapi/introspect.json   | 20 ++--
>>  scripts/qapi/introspect.py | 18 ++
>>  2 files changed, 32 insertions(+), 6 deletions(-)
>> 
>> diff --git a/qapi/introspect.json b/qapi/introspect.json
>> index 39bd303778..250748cd95 100644
>> --- a/qapi/introspect.json
>> +++ b/qapi/introspect.json
>> @@ -142,14 +142,30 @@
>>  #
>>  # Additional SchemaInfo members for meta-type 'enum'.
>>  #
>> -# @values: the enumeration type's values, in no particular order.
>> +# @members: the enum type's members, in no particular order.
>
> Missing a '(since 6.2)' tag.

Yes.

>> +#
>> +# @values: the enumeration type's member names, in no particular order.
>> +#  Redundant with @members.  Just for backward compatibility.
>
> Worth marking as deprecated in this patch, or in a followup?

If we intend to deprecate, we can just as well do it right away.

>>  #
>>  # Values of this type are JSON string on the wire.
>>  #
>>  # Since: 2.5
>>  ##
>>  { 'struct': 'SchemaInfoEnum',
>> -  'data': { 'values': ['str'] } }
>> +  'data': { 'members': [ 'SchemaInfoEnumMember' ],
>> +'values': ['str'] } }
>> +
>> +##
>> +# @SchemaInfoEnumMember:
>> +#
>> +# An object member.
>> +#
>> +# @name: the member's name, as defined in the QAPI schema.
>> +#
>> +# Since: 6.1
>
> 6.2

Whoops!

>> +##
>> +{ 'struct': 'SchemaInfoEnumMember',
>> +  'data': { 'name': 'str' } }
>>
>
> Definitely more flexible.

Re: [PATCH] tools: virsh-snapshot: refactor small functions

2021-09-20 Thread Peter Krempa

On Mon, Sep 20, 2021 at 09:36:25 +0200, Michal Prívozník wrote:
> On 9/17/21 3:23 PM, Kristina Hanicova wrote:
> > This patch includes:
> > * removal of dead code
> > * simplifying nested if conditions
> > * removal of unnecessary variables
> > * usage of "direct" boolean return
> > 
> > Signed-off-by: Kristina Hanicova 
> > ---
> >  tools/virsh-snapshot.c | 43 +++---
> >  1 file changed, 15 insertions(+), 28 deletions(-)
> > 
> > diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> > index 2659b4340d..3bdad03df0 100644
> > --- a/tools/virsh-snapshot.c
> > +++ b/tools/virsh-snapshot.c
> > @@ -771,7 +771,6 @@ virshSnapshotFilter(vshControl *ctl, 
> > virDomainSnapshotPtr snapshot,
> >  g_autofree char *xml = NULL;
> >  g_autoptr(xmlDoc) xmldoc = NULL;
> >  g_autoptr(xmlXPathContext) ctxt = NULL;
> > -int ret = -1;
> >  g_autofree char *state = NULL;
> >  
> >  if (!snapshot)
> > @@ -796,20 +795,17 @@ virshSnapshotFilter(vshControl *ctl, 
> > virDomainSnapshotPtr snapshot,
> >  return -1;
> >  }
> >  if (STREQ(state, "disk-snapshot")) {
> > -ret = ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY |
> > - VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) ==
> > -   (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY |
> > -VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL));
> 
> So the only way this can be true is if both flags are set at the same
> time. Since you're touching this, how about:
> 
> return !!((flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
>   (flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL));

Note that the double negation trick to force a cast to boolean isn't
necessary here, since the result of

 (flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) && (flags & 
VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)

is already a boolean (result of 'A && B')

Re: [PATCH] tools: virsh-snapshot: refactor small functions

2021-09-20 Thread Michal Prívozník

On 9/17/21 3:23 PM, Kristina Hanicova wrote:
> This patch includes:
> * removal of dead code
> * simplifying nested if conditions
> * removal of unnecessary variables
> * usage of "direct" boolean return
> 
> Signed-off-by: Kristina Hanicova 
> ---
>  tools/virsh-snapshot.c | 43 +++---
>  1 file changed, 15 insertions(+), 28 deletions(-)
> 
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index 2659b4340d..3bdad03df0 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -771,7 +771,6 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr 
> snapshot,
>  g_autofree char *xml = NULL;
>  g_autoptr(xmlDoc) xmldoc = NULL;
>  g_autoptr(xmlXPathContext) ctxt = NULL;
> -int ret = -1;
>  g_autofree char *state = NULL;
>  
>  if (!snapshot)
> @@ -796,20 +795,17 @@ virshSnapshotFilter(vshControl *ctl, 
> virDomainSnapshotPtr snapshot,
>  return -1;
>  }
>  if (STREQ(state, "disk-snapshot")) {
> -ret = ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY |
> - VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) ==
> -   (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY |
> -VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL));

So the only way this can be true is if both flags are set at the same
time. Since you're touching this, how about:

return !!((flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
  (flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL));

I find it more readable. I'll change it before pushing.

Reviewed-by: Michal Privoznik 

Michal

Re: [PATCH 8/8] qapi: add blockdev-replace command

2021-09-20 Thread Markus Armbruster

Vladimir Sementsov-Ogievskiy  writes:

> Add command that can add and remove filters.
>
> Key points of functionality:
>
> What the command does is simply replace some BdrvChild.bs by some other
> nodes. The tricky thing is selecting there BdrvChild objects.
> To be able to select any kind of BdrvChild we use a generic parent_id,
> which may be a node-name, or qdev id or block export id. In future we
> may support block jobs.
>
> Any kind of ambiguity leads to error. If we have both device named
> device0 and block export named device0 and they both point to same BDS,
> user can't replace root child of one of these parents. So, to be able
> to do replacements, user should avoid duplicating names in different
> parent namespaces.
>
> So, command allows to replace any single child in the graph.
>
> On the other hand we want to realize a kind of bdrv_replace_node(),
> which works well when we want to replace all parents of some node. For
> this kind of task @parents-mode argument implemented.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  qapi/block-core.json | 78 +
>  block.c  | 82 
>  2 files changed, 160 insertions(+)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 675d8265eb..8059b96341 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -5433,3 +5433,81 @@
>  { 'command': 'blockdev-snapshot-delete-internal-sync',
>'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
>'returns': 'SnapshotInfo' }
> +
> +##
> +# @BlockdevReplaceParentsMode:
> +#
> +# Alternative (to directly set @parent) way to chose parents in
> +# @blockdev-replace
> +#
> +# @exactly-one: Exactly one parent should match a condition, otherwise
> +#   @blockdev-replace fails.
> +#
> +# @all: All matching parents are taken into account. If replacing lead
> +#   to loops in block graph, @blockdev-replace fails.
> +#
> +# @auto: Same as @all, but automatically skip replacing parents if it
> +#leads to loops in block graph.
> +#
> +# Since: 6.2
> +##
> +{ 'enum': 'BlockdevReplaceParentsMode',
> +  'data': ['exactly-one', 'all', 'auto'] }
> +
> +##
> +# @BlockdevReplace:
> +#
> +# Declaration of one replacement.

Replacement of what?  A node in the block graph?

> +#
> +# @parent: id of parent. It may be qdev or block export or simple
> +#  node-name.

It may also be a QOM path, because find_device_state() interprets
arguments starting with '/' as QOM paths.

When is a node name "simple"?

Suggest: It may be a qdev ID, a QOM path, a block export ID, or a node
name.

The trouble is of course that we're merging three separate name spaces.

Aside: a single name space for IDs would be so much saner, but we
screwed that up long ago.

>If id is ambiguous (for example node-name of
> +#  some BDS equals to block export name), blockdev-replace
> +#  fails.

Is there a way out of this situation, or are is replacement simply
impossible then?

>If not specified, blockdev-replace goes through
> +#  @parents-mode scenario, see below. Note, that @parent and
> +#  @parents-mode can't be specified simultaneously.

What if neither is specified?  Hmm, @parents-mode has a default, so
that's what we get.

> +#  If @parent is specified, only one edge is selected. If
> +#  several edges match the condition, blockdev-replace fails.
> +#
> +# @edge: name of the child. If omitted, any child name matches.
> +#
> +# @child: node-name of the child. If omitted, any child matches.
> +# Must be present if @parent is not specified.

Is @child useful when @parent is present?

What's the difference between "name of the child" and "node name of the
child"?

> +#
> +# @parents-mode: declares how to select edge (or edges) when @parent
> +#is omitted. Default is 'one'.

'exactly-one'

Minor combinatorial explosion.  There are four optional arguments, one
of them an enum, and only some combination of argument presence and enum
value are valid.  For a serious review, I'd have to make a table of
combinations, then think through every valid row.

Have you considered making this type a union?  Can turn some of your
semantic constraints into syntactical ones.  Say you turn
BlockdevReplaceParentsMode into a tag enum by adding value 'by-id'.
Then branch 'by-id' has member @parent, and the others don't.

> +#
> +# Since: 6.2
> +#
> +# Examples:
> +#
> +# 1. Change root node of some device.
> +#
> +# Note, that @edge name is omitted, as

Scratch "name".

Odd line break.

> +# devices always has only one child. As well, no need in specifying
> +# old @child.

"the old @child".

> +#
> +# -> { "parent": "device0", "new-child": "some-node-name" }
> +#
> +# 2. Insert copy-before-write filter.
> +#
> +# Assume, after blockdev-add we have block-node 'source', with several
> +# writing parents and

Re: [PATCH 6/8] qdev: realize BlockParentClass

2021-09-20 Thread Markus Armbruster

Vladimir Sementsov-Ogievskiy  writes:

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  softmmu/qdev-monitor.c | 42 ++
>  1 file changed, 42 insertions(+)
>
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 0117989009..2e149aa9b8 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -18,6 +18,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "block/block-parent.h"
>  #include "hw/sysbus.h"
>  #include "monitor/hmp.h"
>  #include "monitor/monitor.h"
> @@ -1023,3 +1024,44 @@ bool qmp_command_available(const QmpCommand *cmd, 
> Error **errp)
>  }
>  return true;
>  }
> +
> +static int qdev_find_child(const char *parent_id, const char *child_name,
> +   BlockDriverState *child_bs,
> +   BdrvChild **child, Error **errp)
> +{
> +int ret;
> +DeviceState *dev;
> +BlockBackend *blk;
> +
> +ret = find_device_state(parent_id, , errp);
> +if (ret <= 0) {
> +return ret;
> +}
> +
> +blk = blk_by_dev(dev);
> +if (!blk || !blk_root(blk)) {
> +error_setg(errp, "Device '%s' does not have a block device backend",
> +   parent_id);
> +return -EINVAL;
> +}
> +
> +if (child_bs && blk_bs(blk) != child_bs) {
> +error_setg(errp, "Root child of device '%s' doesn't match", 
> parent_id);

What is a "root child", and why would a user care?

The contract missing in PATCH 2 leaves me floundering.

> +return -EINVAL;
> +}
> +
> +*child = blk_root(blk);
> +return 1;
> +}
> +
> +BlockParentClass block_parent_qdev = {
> +.name = "qdev",
> +.find_child = qdev_find_child,
> +};
> +
> +static void qdev_monitor_init(void)
> +{
> +block_parent_class_register(_parent_qdev);
> +}
> +
> +block_init(qdev_monitor_init);

56 matches

Mail list logo