Re: [PATCH v4 03/10] migration: migrate 'inc' command option is deprecated.

[Markus Armbruster]

Juan Quintela  writes:

> Markus Armbruster  wrote:
>> Juan Quintela  writes:
>>
>>> Markus Armbruster  wrote:
 Juan Quintela  writes:
>>> So what I want, I want to remove -i/-b in the next version (9.0?).  For
>>> the other, I want to remove it, but I don't care if the code is around
>>> in "deprecated" state for another couple of years if there are still
>>> people that feel that they want it.
>>>
>>> This is the reason that I put a pointer for -i/-b to
>>> @block/@block-incremental.  They are "perfect" replacements.
>>>
>>> I can put here to use blockdev-mirror + NBD, but the replacement is not
>>> so direct.
>>>
>>> Does this make sense?
>>
>> I see where you're coming from.  Now let's change perspective for a
>> minute: what's the purpose of deprecating stuff?
>>
>> We normally deprecate with intent to remove.
>>
>> We don't remove right away, because we promised to first deprecate for a
>> grace period, so users can adjust in an orderly manner.  The deprecation
>> serves as signal "you need to adjust".  The documentation that comes
>> with it should help with the adjustment.  It's commonly of the form "use
>> $alternative instead".  The alternative is often a direct replacement,
>> but not always.  There could even be no replacement at all.  We don't
>> promise replacements, we promise an orderly process, so users can
>> adjust.
>>
>> Sometimes, we don't have firm plans to remove, but are more like "maybe
>> remove when gets in the way".  We could soften the "you need to adjust"
>> signal in documentation, but I doubt that's a good idea.  Regardless,
>> the need to help users adjust remains.
>>
>> Back to your patches.  There are two separate interfaces to block
>> migration, and both are deprecated at the end of the series:
>>
>> 1. Migration parameter @block-incremental & friends
>>
>>Not in the way, content to keep around for longer if it helps users.
>>
>>The deprecation documentation advises to use block-mirror with NBD
>>instead.  All good.
>>
>> 2. QMP migrate parameters @inc and @blk
>>
>>Firm intent to remove as soon as the grace period expires, because
>>it's in the way.
>>
>>The deprecation documentation advises to use interface 1 instead.
>>But that's deprecated, too!
>>
>>Insufficiently careful readers will miss that the replacement is
>>deprecated, and just use it.  Risks surprise when the replacement
>>goes away, too.
>>
>>More careful readers will realize that this advises to use something
>>we elsewhere advise not to use.  Contradiction!  Confusion ensues.
>>
>>On further reflection, these readers might conclude that the
>>*combined* advice is to use block-mirror with NBD instead.  This is
>>correct.
>>
>>So why not tell them?
>>
>>Perhaps you'd like to give more nuanced advice, like "you should move
>>to block-mirror with NBD, but if that's not practical for you, you
>>should at least move to @block-incremental & friends, which will
>>likely stick around for longer."  That's fine.  All I'm asking for is
>>to not make things more confusing than they need to be :)
>>
>> [...]
>
> Telling this in deprecated.rst is enough?  or you want me to put it also
> in the error/warn messages and qapi?

Let's make all of them point to blockdev-mirror with NBD.  If you think
mentioning @block-incremental & friends would be useful in some or all
places would be useful, go ahead, I don't mind.

Re: [PATCH 00/31] qemu: Add accessors for 'storage' and 'format' nodenames and refactor callers ('raw' driver removal part 1)

[Ján Tomko]

On a Monday in 2023, Peter Krempa wrote:

This is a prequel series necessary for removing the dummy 'raw' driver
node if it isn't needed for performance reasons.

Peter Krempa (31):
 qemu: domain: Identify blockjobs by storage nodename in VM status XML
 qemu: block: Refactor logic in qemuBlockStorageSourceGetBlockdevProps
 qemu: block: Rename qemuBlockStorageSourceGetBlockdevProps
 qemu: block: Add accessors for protocol/storage node names
 tests: Use 'storage' layer nodename accessors in tests
 qemuDomainVirStorageSourceFindByNodeName: Use proper accessor
 qemu: block: Use proper accessors for image formatting/creation code
 qemu: domain: Convert the status XML code for 'storage' nodenames to
   new accessors
 qemu: block: Convert disk 'storage' backend JSON props generator to
   new accessors
 qemu: domain: Rework assignment of 'storage' nodenames to use new
   accessors
 qemu: Refactor storage backend attach/detach setup code to use
   'storage' nodename accessors
 qemu: Refactor storage backend 'storage' layer helepr object setup
 qemuDomainGetStatsBlockExportDisk: Use 'storage' node name accessors
 qemuDomainSetBlockThreshold: Use 'storage' node name accessor
 conf: Rename 'nodestorage' field of virStorageSource to
   'nodenamestorage'
 qemu: block: Add accessors for format layer node names
 qemu: block: Add accessors for storage source effective nodename
 qemuBlockStorageSourceGetFormatProps: Use new frontend name accessor
 qemu: backup: Use format nodename accessors
 qemu: blockjob: Use 'format' nodename accessors for job naming
 qemu: block: Use 'format' nodename accessors in '-blockdev' setup code
 qemu: domain: Use 'format' layer node name accessors for nodename
   setup code
 tests: Use 'format' layer nodename accessors in test code
 qemu: Convert disk backend setup code to use 'format' nodename
   accessors
 qemu: driver: Convert disk stats code to use 'format' nodename
   accessors
 qemu: Use 'format' nodename accessors for block dirty bitmap
   operations
 qemu: command: Use 'format' nodename accessors for 'pflash' backend
   setup
 qemu: Convert migration setup code to use 'format' layer node name
   accessors
 qemu: migration: Use 'format' nodename accessors in dirty bitmap
   migration
 qemu: driver: Use 'format' nodename accessors for disk resize
 conf: Rename 'nodeformat' field of virStorageSource to
   'nodenameformat'

src/conf/storage_source_conf.c|   8 +-
src/conf/storage_source_conf.h|   4 +-
src/qemu/qemu_backup.c|   8 +-
src/qemu/qemu_block.c | 238 --
src/qemu/qemu_block.h |  25 +-
src/qemu/qemu_blockjob.c  |  24 +-
src/qemu/qemu_checkpoint.c|   9 +-
src/qemu/qemu_command.c   |  12 +-
src/qemu/qemu_domain.c|  68 ++---
src/qemu/qemu_driver.c|  25 +-
src/qemu/qemu_migration.c |  17 +-
src/qemu/qemu_migration_cookie.c  |   5 +-
src/qemu/qemu_nbdkit.c|   4 +-
src/qemu/qemu_process.c   |   2 +-
src/qemu/qemu_snapshot.c  |   6 +-
tests/qemublocktest.c |  16 +-
tests/qemumonitorjsontest.c   |   4 +-
.../blockjob-blockdev-in.xml  |  12 +-
tests/qemuxml2argvtest.c  |   3 +-
19 files changed, 314 insertions(+), 176 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

[libvirt PATCH v2] hypervisor: Move interface mgmt methods to hypervisor

[Praveen K Paladugu]

Move guest interface management methods from qemu to hypervisor. These
methods will be shared by networking support in ch driver.

Signed-off-by: Praveen K Paladugu 
---
 po/POTFILES   |   1 +
 src/hypervisor/domain_interface.c | 280 ++
 src/hypervisor/domain_interface.h |  39 +
 src/hypervisor/meson.build|   1 +
 src/libvirt_private.syms  |   6 +
 src/qemu/qemu_command.c   |   7 +-
 src/qemu/qemu_hotplug.c   |   3 +-
 src/qemu/qemu_interface.c | 246 +-
 src/qemu/qemu_interface.h |   6 -
 src/qemu/qemu_process.c   |   3 +-
 10 files changed, 343 insertions(+), 249 deletions(-)
 create mode 100644 src/hypervisor/domain_interface.c
 create mode 100644 src/hypervisor/domain_interface.h

diff --git a/po/POTFILES b/po/POTFILES
index 3a51aea5cb..023c041f61 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -92,6 +92,7 @@ src/hyperv/hyperv_util.c
 src/hyperv/hyperv_wmi.c
 src/hypervisor/domain_cgroup.c
 src/hypervisor/domain_driver.c
+src/hypervisor/domain_interface.c
 src/hypervisor/virhostdev.c
 src/interface/interface_backend_netcf.c
 src/interface/interface_backend_udev.c
diff --git a/src/hypervisor/domain_interface.c 
b/src/hypervisor/domain_interface.c
new file mode 100644
index 00..592c4253df
--- /dev/null
+++ b/src/hypervisor/domain_interface.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (C) 2015-2016 Red Hat, Inc.
+ * Copyright IBM Corp. 2014
+ *
+ * domain_interface.c: methods to manage guest/domain interfaces
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "virconftypes.h"
+#include "virmacaddr.h"
+#include "virnetdevtap.h"
+#include "domain_audit.h"
+#include "domain_conf.h"
+#include "domain_interface.h"
+#include "domain_nwfilter.h"
+#include "viralloc.h"
+#include "virebtables.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "virnetdevbridge.h"
+#include "network_conf.h"
+
+#define VIR_FROM_THIS VIR_FROM_INTERFACE
+
+VIR_LOG_INIT("interface.interface_connect");
+
+bool
+virDomainInterfaceIsVnetCompatModel(const virDomainNetDef *net)
+{
+return (virDomainNetIsVirtioModel(net) ||
+net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
+net->model == VIR_DOMAIN_NET_MODEL_IGB ||
+net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
+}
+
+/* virDomainInterfaceEthernetConnect:
+ * @def: the definition of the VM
+ * @driver: qemu driver data
+ * @net: pointer to the VM's interface description
+ * @tapfd: array of file descriptor return value for the new device
+ * @tapfdsize: number of file descriptors in @tapfd
+ *
+ * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_ETHERNET
+ * (i.e. if the connection is made with a tap device)
+ */
+int
+virDomainInterfaceEthernetConnect(virDomainDef *def,
+virDomainNetDef *net,
+ebtablesContext *ebtables,
+bool macFilter,
+bool privileged,
+int *tapfd,
+size_t tapfdSize)
+{
+virMacAddr tapmac;
+int ret = -1;
+unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
+bool template_ifname = false;
+const char *tunpath = "/dev/net/tun";
+const char *auditdev = tunpath;
+
+if (net->backend.tap) {
+tunpath = net->backend.tap;
+if (!privileged) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("cannot use custom tap device in session mode"));
+goto cleanup;
+}
+}
+VIR_WARN("PPK: interfaceEthernetConnect Called\n");
+if (virDomainInterfaceIsVnetCompatModel(net))
+tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
+
+if (net->managed_tap == VIR_TRISTATE_BOOL_NO) {
+if (!net->ifname) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("target dev must be supplied when managed='no'"));
+goto cleanup;
+}
+if (virNetDevExists(net->ifname) != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("target managed='no' but specified dev doesn't 
exist"));
+goto cleanup;
+}
+
+tap

Re: [libvirt PATCH] hypervisor: Move interface mgmt methods to hypervisor

[Praveen Paladugu]

On Sun, Oct 15, 2023 at 03:03:40PM -0400, Laine Stump wrote:
> On 10/12/23 3:37 PM, Praveen K Paladugu wrote:
> >Move guest interface management methods from qemu to hypervisor. These
> >methods will be shared by networking support in ch driver.
> >
> >Signed-off-by: Praveen K Paladugu 
> >---
> >  po/POTFILES   |   1 +
> >  src/hypervisor/domain_interface.c | 279 ++
> >  src/hypervisor/domain_interface.h |  38 
> >  src/hypervisor/meson.build|   1 +
> >  src/libvirt_private.syms  |   6 +
> >  src/qemu/qemu_command.c   |   7 +-
> >  src/qemu/qemu_hotplug.c   |   3 +-
> >  src/qemu/qemu_interface.c | 246 +-
> >  src/qemu/qemu_interface.h |   6 -
> >  src/qemu/qemu_process.c   |   3 +-
> >  10 files changed, 341 insertions(+), 249 deletions(-)
> >  create mode 100644 src/hypervisor/domain_interface.c
> >  create mode 100644 src/hypervisor/domain_interface.h
> >
> >diff --git a/po/POTFILES b/po/POTFILES
> >index 3a51aea5cb..023c041f61 100644
> >--- a/po/POTFILES
> >+++ b/po/POTFILES
> >@@ -92,6 +92,7 @@ src/hyperv/hyperv_util.c
> >  src/hyperv/hyperv_wmi.c
> >  src/hypervisor/domain_cgroup.c
> >  src/hypervisor/domain_driver.c
> >+src/hypervisor/domain_interface.c
> >  src/hypervisor/virhostdev.c
> >  src/interface/interface_backend_netcf.c
> >  src/interface/interface_backend_udev.c
> >diff --git a/src/hypervisor/domain_interface.c 
> >b/src/hypervisor/domain_interface.c
> >new file mode 100644
> >index 00..b01b6ee767
> >--- /dev/null
> >+++ b/src/hypervisor/domain_interface.c
> >@@ -0,0 +1,279 @@
> >+/*
> >+ * Copyright Microsoft Corp. 2023
> 
> I haven't had time to look through the rest of this yet (although it
> seems pretty straightforward, and I think it might have been me who
> suggested it in the first place :-)), I did want to bring up this
> one topic now:
> 
> 
> Best practices for the copyright notice in a new file have changed
> over the years (for example, we no longer list the "Author" of new
> files and even removed Author from existing files, since a more
> accurate and complete version of that info is all available from
> git), and I haven't paid close attention, but since this entire file
> is comprised of functions that were just moved from an existing file
> and renamed (rather than actual new code), I don't think it's
> appropriate for it to erase all trace of the original copyright
> holder and simply assign the copyright entirely to Microsoft.
> 
> What are others' opinions on this?
> 
> 
Thanks for the pointing this out. It was a miss on my part. I will send
an updated version with the original copyrights included.

- Praveen
> >+ *
> >+ * domain_interface.c: methods to manage guest/domain interfaces
> >+ *
> >+ * This library is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU Lesser General Public
> >+ * License as published by the Free Software Foundation; either
> >+ * version 2.1 of the License, or (at your option) any later version.
> >+ *
> >+ * This library is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >+ * Lesser General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU Lesser General Public
> >+ * License along with this library.  If not, see
> >+ * .
> >+ */
> >+
> >+#include 
> >+
> >+#include "virmacaddr.h"
> >+#include "virnetdevtap.h"
> >+#include "virconftypes.h"
> >+#include "domain_conf.h"
> >+#include "domain_interface.h"
> >+#include "domain_nwfilter.h"
> >+#include "domain_audit.h"
> >+#include "virebtables.h"
> >+#include "virlog.h"
> >+#include "virfile.h"
> >+#include "viralloc.h"
> >+#include "virnetdevbridge.h"
> >+#include "network_conf.h"
> >+
> >+#define VIR_FROM_THIS VIR_FROM_INTERFACE
> >+
> >+VIR_LOG_INIT("interface.interface_connect");
> >+
> >+bool
> >+virDomainInterfaceIsVnetCompatModel(const virDomainNetDef *net)
> >+{
> >+return (virDomainNetIsVirtioModel(net) ||
> >+net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
> >+net->model == VIR_DOMAIN_NET_MODEL_IGB ||
> >+net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
> >+}
> >+
> >+/* virDomainInterfaceEthernetConnect:
> >+ * @def: the definition of the VM
> >+ * @driver: qemu driver data
> >+ * @net: pointer to the VM's interface description
> >+ * @tapfd: array of file descriptor return value for the new device
> >+ * @tapfdsize: number of file descriptors in @tapfd
> >+ *
> >+ * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_ETHERNET
> >+ * (i.e. if the connection is made with a tap device)
> >+ */
> >+int
> >+virDomainInterfaceEthernetConnect(virDomainDef *def,
> >+virDomainNetDef *net,
> >+e

Re: [PATCH 18/31] qemuBlockStorageSourceGetFormatProps: Use new frontend name accessor

[Ján Tomko]

On a Monday in 2023, Peter Krempa wrote:

---
src/qemu/qemu_block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



https://libvirt.org/hacking.html#developer-certificate-of-origin

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH 04/31] qemu: block: Add accessors for protocol/storage node names

[Ján Tomko]

On a Monday in 2023, Peter Krempa wrote:

Introduce a set of accessors, which return node names based on
semantics. This will allow to us to modify how we setup the backing
chain in cases when e.g. the format driver can be omitted, without
breaking all the code.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_block.c | 48 +++
src/qemu/qemu_block.h | 11 ++
2 files changed, 59 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 07bc8ede76..0c9460f678 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -52,6 +52,54 @@ qemuBlockNodeNameValidate(const char *nn)
}


+/**
+ * qemuBlockStorageSourceSetStorageNodename:
+ * @src: virStorageSource to set the storage nodename
+ * @nodename: The node name to set (stolen)


But that's stealing!


+ *
+ * Sets @nodename as the storage node name of @src. Using NULL @nodename clears
+ * the nodename. @src takes ownership of @nodename.
+ */


Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH 01/31] qemu: domain: Identify blockjobs by storage nodename in VM status XML

[Ján Tomko]

On a Monday in 2023, Peter Krempa wrote:

Use the node name of the storage access driver to identify the block job
volumes. This will prepare the blockjob code to the possibility that the


s/to the/for the/


format layer may be missing. Our lookup code can find either of them,
thus we can safely switch.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_domain.c   |  8 
tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 12 ++--
2 files changed, 10 insertions(+), 10 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH v2] tests: qemucapabilitiesdata: Add test data for qemu-8.2 dev cycle on x86_64

[Ján Tomko]

On a Monday in 2023, Peter Krempa wrote:

Add the test data based on v8.1.0-1639-g63011373ad

Notable changes in comparison with qemu-8.1 release:
- new 8.2 machine types added
- removed machine types: 'pc-i440fx-1.4', 'pc-i440fx-1.5', 'pc-i440fx-1.6', 
'pc-i440fx-1.7'
- 'rom' parameter for 'memory-backend-file' added
- 'qemu-text-console', 'qemu-graphic-console',
  'qemu-fixed-text-console' QOM types added
- 'qemu-console' -object removed
- 'ufs-lu', 'ufs' devices added
- 'sd-card-spi' device added
- 'cryptodev-backend-lkcf' added
- 'calc-dirty-rate' paramters 'calc-time-unit' added
- 'guest_uso4', 'guest_uso6' 'host_uso' options for 'virtio-net-pci' added
- new cpu flags: 'vmx-any-errcode', 'gds-no', 'vmx-complex',
  'vmx-enable-user-wait-pause'

Signed-off-by: Peter Krempa 
---

v2:
- another 600 commits in qemu

.../domaincapsdata/qemu_8.2.0-q35.x86_64.xml  |   288 +
.../domaincapsdata/qemu_8.2.0-tcg.x86_64.xml  |   289 +
tests/domaincapsdata/qemu_8.2.0.x86_64.xml|   288 +
.../caps_8.2.0_x86_64.replies | 42259 
.../caps_8.2.0_x86_64.xml |  3788 ++
5 files changed, 46912 insertions(+)
create mode 100644 tests/domaincapsdata/qemu_8.2.0-q35.x86_64.xml
create mode 100644 tests/domaincapsdata/qemu_8.2.0-tcg.x86_64.xml
create mode 100644 tests/domaincapsdata/qemu_8.2.0.x86_64.xml
create mode 100644 tests/qemucapabilitiesdata/caps_8.2.0_x86_64.replies
create mode 100644 tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH 0/4] Misc cleanups

[Ján Tomko]

On a Monday in 2023, Michal Privoznik wrote:

*** BLURB HERE ***

Michal Prívozník (4):
 lib: Replace virBuildPath() with g_build_filename()
 virfile: Drop virBuildPathInternal()
 virSecretLoadAllConfigs: Use g_autofree for @path
 virSecretLoad: Simplify cleanup path

docs/kbase/internals/command.rst |  2 +-
src/conf/virsecretobj.c  | 15 ++-
src/libvirt_private.syms |  1 -
src/util/virfcp.c|  2 +-
src/util/virfile.c   | 22 --
src/util/virfile.h   |  5 -
src/util/virhook.c   |  4 ++--
src/util/virpci.c|  4 ++--
8 files changed, 12 insertions(+), 43 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

[PATCH 13/31] qemuDomainGetStatsBlockExportDisk: Use 'storage' node name accessors

[Peter Krempa]

In all cases we want to probe stats from the 'storage' layer as we're
interested in the 'threshold' value, which we set there.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f3c9730cd8..e2fe2016af 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17349,7 +17349,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef 
*disk,
 if (QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName) {
 frontendalias = QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName;
 backendalias = n->nodeformat;
-backendstoragealias = n->nodestorage;
+backendstoragealias = qemuBlockStorageSourceGetStorageNodename(n);
 } else {
 /* alias may be NULL if the VM is not running */
 if (disk->info.alias &&
@@ -17408,7 +17408,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef 
*disk,
stats) < 0)
 return -1;

-if 
(qemuDomainGetStatsBlockExportBackendStorage(disk->mirror->nodestorage,
+if 
(qemuDomainGetStatsBlockExportBackendStorage(qemuBlockStorageSourceGetStorageNodename(disk->mirror),
 stats, *recordnr,
 params) < 0)
 return -1;
@@ -17437,7 +17437,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef 
*disk,
stats) < 0)
 return -1;

-if 
(qemuDomainGetStatsBlockExportBackendStorage(backupdisk->store->nodestorage,
+if 
(qemuDomainGetStatsBlockExportBackendStorage(qemuBlockStorageSourceGetStorageNodename(backupdisk->store),
 stats, 
*recordnr,
 params) < 
0)
 return -1;
-- 
2.41.0

[PATCH 17/31] qemu: block: Add accessors for storage source effective nodename

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 14 ++
 src/qemu/qemu_block.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index cba1fb1c1e..10c2c0104b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -86,6 +86,20 @@ qemuBlockStorageSourceSetFormatNodename(virStorageSource 
*src,
 }


+/**
+ * qemuBlockStorageSourceGetEffectiveNodename:
+ * @src: virStorageSource to get the effective nodename of
+ *
+ * Gets the nodename that exposes the guest visible data. This function always
+ * returns a name.
+ */
+const char *
+qemuBlockStorageSourceGetEffectiveNodename(virStorageSource *src)
+{
+return src->nodeformat;
+}
+
+
 /**
  * qemuBlockStorageSourceGetEffectiveStorageNodename:
  * @src: virStorageSource to get the effective nodename of
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 6ed0aa85b2..7008a4e7da 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -43,6 +43,9 @@ qemuBlockStorageSourceGetStorageNodename(virStorageSource 
*src);
 const char *
 qemuBlockStorageSourceGetFormatNodename(virStorageSource *src);

+const char *
+qemuBlockStorageSourceGetEffectiveNodename(virStorageSource *src);
+

 typedef struct qemuBlockNodeNameBackingChainData 
qemuBlockNodeNameBackingChainData;
 struct qemuBlockNodeNameBackingChainData {
-- 
2.41.0

[PATCH 11/31] qemu: Refactor storage backend attach/detach setup code to use 'storage' nodename accessors

[Peter Krempa]

Refactor the code settin up data structures used to attach/detach disks
and SCSI hostdevs.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c   | 7 ---
 src/qemu/qemu_command.c | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 1fc36569a9..7355cb0b5e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1484,7 +1484,7 @@ 
qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSource *src,
  
backendpropsflags)))
 return NULL;

-data->storageNodeName = src->nodestorage;
+data->storageNodeName = qemuBlockStorageSourceGetStorageNodename(src);
 data->formatNodeName = src->nodeformat;

 if (qemuBlockStorageSourceNeedsStorageSliceLayer(src)) {
@@ -1705,7 +1705,7 @@ qemuBlockStorageSourceDetachPrepare(virStorageSource *src)

 data->formatNodeName = src->nodeformat;
 data->formatAttached = true;
-data->storageNodeName = src->nodestorage;
+data->storageNodeName = qemuBlockStorageSourceGetStorageNodename(src);
 data->storageAttached = true;

 /* 'raw' format doesn't need the extra 'raw' layer when slicing, thus
@@ -1899,7 +1899,8 @@ qemuBlockStorageSourceDetachOneBlockdev(virDomainObj *vm,
 ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm), src->nodeformat);

 if (ret == 0)
-ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm), 
src->nodestorage);
+ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm),
+ 
qemuBlockStorageSourceGetStorageNodename(src));

 qemuDomainObjExitMonitor(vm);

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8a7b80719f..40de712c61 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5049,7 +5049,7 @@ qemuBuildHostdevSCSIDetachPrepare(virDomainHostdevDef 
*hostdev,
 }

 srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
-ret->storageNodeName = src->nodestorage;
+ret->storageNodeName = qemuBlockStorageSourceGetStorageNodename(src);
 ret->storageAttached = true;

 if (srcpriv && srcpriv->secinfo)
@@ -5083,8 +5083,8 @@ qemuBuildHostdevSCSIAttachPrepare(virDomainHostdevDef 
*hostdev,
 return NULL;
 }

-ret->storageNodeName = src->nodestorage;
-*backendAlias = src->nodestorage;
+ret->storageNodeName = qemuBlockStorageSourceGetStorageNodename(src);
+*backendAlias = qemuBlockStorageSourceGetStorageNodename(src);

 if (!(ret->storageProps = qemuBlockStorageSourceGetBackendProps(src,
 
QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_SKIP_UNMAP)))
-- 
2.41.0

[PATCH 08/31] qemu: domain: Convert the status XML code for 'storage' nodenames to new accessors

[Peter Krempa]

Use the new accessors in the private XML formatters and parsers and the
recovery code.

Specifically in all instances we use the proper (not effective) storage
nodename. In the virStorageSource private data it is what we need to
store. In blockjobs status XML it simply serves us to find the
appropriate 'virStorageSource' struct so using the storage layer node
name is simpler.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f891defa91..78dc99d243 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2000,7 +2000,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
 int enccount;
 xmlNodePtr nbdkitnode = NULL;

-src->nodestorage = 
virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt);
+qemuBlockStorageSourceSetStorageNodename(src, 
virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt));
 src->nodeformat = 
virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt);
 src->tlsAlias = virXPathString("string(./objects/TLSx509/@alias)", ctxt);

@@ -2111,7 +2111,7 @@ qemuStorageSourcePrivateDataFormat(virStorageSource *src,
 g_auto(virBuffer) objectsChildBuf = VIR_BUFFER_INIT_CHILD(buf);
 g_auto(virBuffer) fdsetsChildBuf = VIR_BUFFER_INIT_CHILD(buf);

-virBufferEscapeString(&nodenamesChildBuf, "\n", src->nodestorage);
+virBufferEscapeString(&nodenamesChildBuf, "\n", qemuBlockStorageSourceGetStorageNodename(src));
 virBufferEscapeString(&nodenamesChildBuf, "\n", src->nodeformat);

 if (src->sliceStorage)
@@ -2288,13 +2288,16 @@ qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobData 
*job,
 g_auto(virBuffer) disabledBitmapsBuf = VIR_BUFFER_INIT_CHILD(buf);

 if (job->data.commit.base)
-virBufferAsprintf(buf, "\n", 
job->data.commit.base->nodestorage);
+virBufferAsprintf(buf, "\n",
+  
qemuBlockStorageSourceGetStorageNodename(job->data.commit.base));

 if (job->data.commit.top)
-virBufferAsprintf(buf, "\n", 
job->data.commit.top->nodestorage);
+virBufferAsprintf(buf, "\n",
+  
qemuBlockStorageSourceGetStorageNodename(job->data.commit.top));

 if (job->data.commit.topparent)
-virBufferAsprintf(buf, "\n", 
job->data.commit.topparent->nodestorage);
+virBufferAsprintf(buf, "\n",
+  
qemuBlockStorageSourceGetStorageNodename(job->data.commit.topparent));

 if (job->data.commit.deleteCommittedImages)
 virBufferAddLit(buf, "\n");
@@ -2357,7 +2360,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void 
*payload,
 switch ((qemuBlockJobType) job->type) {
 case QEMU_BLOCKJOB_TYPE_PULL:
 if (job->data.pull.base)
-virBufferAsprintf(&childBuf, "\n", 
job->data.pull.base->nodestorage);
+virBufferAsprintf(&childBuf, "\n",
+  
qemuBlockStorageSourceGetStorageNodename(job->data.pull.base));
 break;

 case QEMU_BLOCKJOB_TYPE_COMMIT:
@@ -6060,8 +6064,8 @@ 
qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias(virDomainHostdevDef *host
 return -1;
 }

-if (!src->nodestorage)
-src->nodestorage = g_strdup_printf("libvirt-%s-backend", 
hostdev->info->alias);
+if (!qemuBlockStorageSourceGetStorageNodename(src))
+qemuBlockStorageSourceSetStorageNodename(src, 
g_strdup_printf("libvirt-%s-backend", hostdev->info->alias));

 return 0;
 }
-- 
2.41.0

[PATCH 26/31] qemu: Use 'format' nodename accessors for block dirty bitmap operations

[Peter Krempa]

In most cases the bitmap operations are relevant only on qcow2 images
thus the 'format' layer will be present. Although in certain specific
cases temporary bitmaps can be created on top of other images as well,
thus we use the 'effective' bitmap name in all cases for bitmap
operations.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c  | 27 ++-
 src/qemu/qemu_blockjob.c   |  4 ++--
 src/qemu/qemu_checkpoint.c |  9 +
 src/qemu/qemu_driver.c |  7 ---
 src/qemu/qemu_snapshot.c   |  6 --
 5 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 4c1a711dd3..edc8edcb70 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2807,7 +2807,8 @@ qemuBlockNamedNodeDataGetBitmapByName(GHashTable 
*blockNamedNodeData,
 qemuBlockNamedNodeData *nodedata;
 size_t i;

-if (!(nodedata = virHashLookup(blockNamedNodeData, src->nodeformat)))
+if (!(nodedata = virHashLookup(blockNamedNodeData,
+   
qemuBlockStorageSourceGetEffectiveNodename(src
 return NULL;

 for (i = 0; i < nodedata->nbitmaps; i++) {
@@ -2863,7 +2864,7 @@ qemuBlockGetBitmapMergeActionsGetBitmaps(virStorageSource 
*topsrc,
 /* for now it doesn't make sense to consider bitmaps which are not present
  * in @topsrc as we can't recreate a bitmap for a layer if it's missing */

-if (!(entry = virHashLookup(blockNamedNodeData, topsrc->nodeformat)))
+if (!(entry = virHashLookup(blockNamedNodeData, 
qemuBlockStorageSourceGetEffectiveNodename(topsrc
 return NULL;

 for (i = 0; i < entry->nbitmaps; i++) {
@@ -2972,7 +2973,7 @@ qemuBlockGetBitmapMergeActions(virStorageSource *topsrc,
 granularity = bitmap->granularity;

 if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(merge,
- n->nodeformat,
+ 
qemuBlockStorageSourceGetEffectiveNodename(n),
  bitmap->name) 
< 0)
 return -1;
 }
@@ -2982,7 +2983,7 @@ qemuBlockGetBitmapMergeActions(virStorageSource *topsrc,
  target, 
curbitmap))) {

 if (qemuMonitorTransactionBitmapAdd(act,
-target->nodeformat,
+
qemuBlockStorageSourceGetEffectiveNodename(target),
 mergebitmapname,
 mergebitmappersistent,
 mergebitmapdisabled,
@@ -2992,18 +2993,18 @@ qemuBlockGetBitmapMergeActions(virStorageSource *topsrc,

 if (writebitmapsrc &&
 qemuMonitorTransactionBitmapMergeSourceAddBitmap(merge,
- 
writebitmapsrc->nodeformat,
+ 
qemuBlockStorageSourceGetEffectiveNodename(writebitmapsrc),
  
"libvirt-tmp-activewrite") < 0)
 return -1;

-if (qemuMonitorTransactionBitmapMerge(act, target->nodeformat,
+if (qemuMonitorTransactionBitmapMerge(act, 
qemuBlockStorageSourceGetEffectiveNodename(target),
   mergebitmapname, &merge) < 0)
 return -1;
 }

  done:
 if (writebitmapsrc &&
-qemuMonitorTransactionBitmapRemove(act, writebitmapsrc->nodeformat,
+qemuMonitorTransactionBitmapRemove(act, 
qemuBlockStorageSourceGetEffectiveNodename(writebitmapsrc),
"libvirt-tmp-activewrite") < 0)
 return -1;

@@ -3578,8 +3579,8 @@ qemuBlockCommit(virDomainObj *vm,
 rc = qemuMonitorBlockCommit(priv->mon,
 qemuDomainDiskGetTopNodename(disk),
 job->name,
-topSource->nodeformat,
-baseSource->nodeformat,
+
qemuBlockStorageSourceGetEffectiveNodename(topSource),
+
qemuBlockStorageSourceGetEffectiveNodename(baseSource),
 backingPath, bandwidth,
 autofinalize);

@@ -3663,7 +3664,7 @@ qemuBlockPivot(virDomainObj *vm,
 bitmapactions = virJSONValueNewArray();

 if (qemuMonitorTransactionBitmapAdd(bitmapactions,
-disk->mirror->nodeformat,
+
qemuBlockStorageSourceGetEffectiveNodename(disk->mirror),
 "libvirt-tmp-activewrite",
   

[PATCH 21/31] qemu: block: Use 'format' nodename accessors in '-blockdev' setup code

[Peter Krempa]

Convert the main -blockdev JSON object setup code to use the new
accessors. In these we use mainly the real 'format' layer node name.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0fa5b6e55d..852028f014 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1282,7 +1282,7 @@ 
qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSource *src)
 
src->detect_zeroes);
 g_autoptr(virJSONValue) props = NULL;

-if (qemuBlockNodeNameValidate(src->nodeformat) < 0)
+if 
(qemuBlockNodeNameValidate(qemuBlockStorageSourceGetFormatNodename(src)) < 0)
 return NULL;

 if (src->discard)
@@ -1296,7 +1296,7 @@ 
qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSource *src)
  */

 if (virJSONValueObjectAdd(&props,
-  "s:node-name", src->nodeformat,
+  "s:node-name", 
qemuBlockStorageSourceGetFormatNodename(src),
   "b:read-only", src->readonly,
   "S:discard", discard,
   "S:detect-zeroes", detectZeroes,
@@ -1530,7 +1530,7 @@ 
qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSource *src,
 return NULL;

 data->storageNodeName = qemuBlockStorageSourceGetStorageNodename(src);
-data->formatNodeName = src->nodeformat;
+data->formatNodeName = qemuBlockStorageSourceGetFormatNodename(src);

 if (qemuBlockStorageSourceNeedsStorageSliceLayer(src)) {
 if (!(data->storageSliceProps = 
qemuBlockStorageSourceGetBlockdevStorageSliceProps(src)))
@@ -1748,7 +1748,7 @@ qemuBlockStorageSourceDetachPrepare(virStorageSource *src)

 data = g_new0(qemuBlockStorageSourceAttachData, 1);

-data->formatNodeName = src->nodeformat;
+data->formatNodeName = qemuBlockStorageSourceGetFormatNodename(src);
 data->formatAttached = true;
 data->storageNodeName = qemuBlockStorageSourceGetStorageNodename(src);
 data->storageAttached = true;
@@ -1941,7 +1941,8 @@ qemuBlockStorageSourceDetachOneBlockdev(virDomainObj *vm,
 if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
 return -1;

-ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm), src->nodeformat);
+ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm),
+ qemuBlockStorageSourceGetFormatNodename(src));

 if (ret == 0)
 ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm),
@@ -1959,8 +1960,8 @@ qemuBlockSnapshotAddBlockdev(virJSONValue *actions,
  virStorageSource *newsrc)
 {
 return qemuMonitorTransactionSnapshotBlockdev(actions,
-  disk->src->nodeformat,
-  newsrc->nodeformat);
+  
qemuBlockStorageSourceGetEffectiveNodename(disk->src),
+  
qemuBlockStorageSourceGetFormatNodename(newsrc));
 }


-- 
2.41.0

[PATCH 23/31] tests: Use 'format' layer nodename accessors in test code

[Peter Krempa]

The test code cares mostly about the actual layer nodenames thus,
appropriate accessors are used.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c   | 10 +-
 tests/qemumonitorjsontest.c |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 9a72a67ce2..addd646071 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -241,7 +241,7 @@ testQemuDiskXMLToJSONFakeSecrets(virStorageSource *src)
 srcpriv->encinfo[0] = g_new0(qemuDomainSecretInfo, 1);

 srcpriv->encinfo[0]->alias = g_strdup_printf("%s-encalias",
- NULLSTR(src->nodeformat));
+ 
qemuBlockStorageSourceGetFormatNodename(src));
 srcpriv->enccount = 1;
 }

@@ -653,7 +653,7 @@ testQemuBitmapListPrint(const char *title,

 for (; next; next = next->next) {
 virStorageSource *src = next->data;
-virBufferAsprintf(buf, "%s\n", src->nodeformat);
+virBufferAsprintf(buf, "%s\n", 
qemuBlockStorageSourceGetFormatNodename(src));
 }
 }

@@ -668,7 +668,7 @@ testQemuBackupIncrementalBitmapCalculateGetFakeImage(size_t 
idx)
ret->format = VIR_STORAGE_FILE_QCOW2;
ret->path = g_strdup_printf("/image%zu", idx);
qemuBlockStorageSourceSetStorageNodename(ret, 
g_strdup_printf("libvirt-%zu-storage", idx));
-   ret->nodeformat = g_strdup_printf("libvirt-%zu-format", idx);
+   qemuBlockStorageSourceSetFormatNodename(ret, 
g_strdup_printf("libvirt-%zu-format", idx));

return ret;
 }
@@ -741,7 +741,7 @@ testQemuBackupIncrementalBitmapCalculate(const void *opaque)
 }

 target = virStorageSourceNew();
-target->nodeformat = g_strdup_printf("target_node");
+qemuBlockStorageSourceSetFormatNodename(target, 
g_strdup_printf("target_node"));

 if (qemuBackupDiskPrepareOneBitmapsChain(data->chain,
  target,
@@ -872,7 +872,7 @@ testQemuBlockBitmapBlockcopy(const void *opaque)
 g_autoptr(virStorageSource) fakemirror = virStorageSourceNew();
 g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;

-fakemirror->nodeformat = g_strdup("mirror-format-node");
+qemuBlockStorageSourceSetFormatNodename(fakemirror, 
g_strdup("mirror-format-node"));

 expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
  blockcopyPrefix, data->name);
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 6b82e3841b..6293b416bd 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2600,7 +2600,7 @@ testQemuMonitorJSONBlockdevReopen(const void *opaque)

 src->format = VIR_STORAGE_FILE_QCOW2;
 src->readonly = true;
-src->nodeformat = g_strdup("test node");
+qemuBlockStorageSourceSetFormatNodename(src, g_strdup("test node"));
 qemuBlockStorageSourceSetStorageNodename(src, g_strdup("backing 
nodename"));
 src->backingStore = virStorageSourceNew();

-- 
2.41.0

[PATCH 14/31] qemuDomainSetBlockThreshold: Use 'storage' node name accessor

[Peter Krempa]

We need to keep setting the block threshold on the real storage layer
per semantics of the API. Use the appropriate accessor.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e2fe2016af..6d95711cbc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18682,7 +18682,7 @@ qemuDomainSetBlockThreshold(virDomainPtr dom,
 goto endjob;
 }

-nodename = g_strdup(src->nodestorage);
+nodename = g_strdup(qemuBlockStorageSourceGetStorageNodename(src));

 qemuDomainObjEnterMonitor(vm);
 rc = qemuMonitorSetBlockThreshold(priv->mon, nodename, threshold);
-- 
2.41.0

[PATCH 20/31] qemu: blockjob: Use 'format' nodename accessors for job naming

[Peter Krempa]

Use the effective nodename for naming the job as we use that one now.
It doesn't matter too much which one we pick, because it's used just for
the name of the job, which we preserve in the status XML.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 12f3d59df5..cb9948ae2a 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -252,7 +252,8 @@ qemuBlockJobDiskNewPull(virDomainObj *vm,
 unsigned int jobflags)
 {
 g_autoptr(qemuBlockJobData) job = NULL;
-g_autofree char *jobname = g_strdup_printf("pull-%s-%s", disk->dst, 
disk->src->nodeformat);
+g_autofree char *jobname = g_strdup_printf("pull-%s-%s", disk->dst,
+   
qemuBlockStorageSourceGetEffectiveNodename(disk->src));

 if (!(job = qemuBlockJobDataNew(QEMU_BLOCKJOB_TYPE_PULL, jobname)))
 return NULL;
@@ -278,7 +279,8 @@ qemuBlockJobDiskNewCommit(virDomainObj *vm,
   unsigned int jobflags)
 {
 g_autoptr(qemuBlockJobData) job = NULL;
-g_autofree char *jobname = g_strdup_printf("commit-%s-%s", disk->dst, 
top->nodeformat);
+g_autofree char *jobname = g_strdup_printf("commit-%s-%s", disk->dst,
+   
qemuBlockStorageSourceGetEffectiveNodename(top));
 qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT;

 if (topparent == NULL)
@@ -309,7 +311,7 @@ qemuBlockJobNewCreate(virDomainObj *vm,
 {
 g_autoptr(qemuBlockJobData) job = NULL;
 g_autofree char *jobname = NULL;
-const char *nodename = src->nodeformat;
+const char *nodename = qemuBlockStorageSourceGetEffectiveNodename(src);

 if (storage)
 nodename = qemuBlockStorageSourceGetStorageNodename(src);
@@ -340,7 +342,8 @@ qemuBlockJobDiskNewCopy(virDomainObj *vm,
 unsigned int jobflags)
 {
 g_autoptr(qemuBlockJobData) job = NULL;
-g_autofree char *jobname = g_strdup_printf("copy-%s-%s", disk->dst, 
disk->src->nodeformat);
+g_autofree char *jobname = g_strdup_printf("copy-%s-%s", disk->dst,
+   
qemuBlockStorageSourceGetEffectiveNodename(disk->src));

 if (!(job = qemuBlockJobDataNew(QEMU_BLOCKJOB_TYPE_COPY, jobname)))
 return NULL;
@@ -368,7 +371,8 @@ qemuBlockJobDiskNewBackup(virDomainObj *vm,
 g_autoptr(qemuBlockJobData) job = NULL;
 g_autofree char *jobname = NULL;

-jobname = g_strdup_printf("backup-%s-%s", disk->dst, 
disk->src->nodeformat);
+jobname = g_strdup_printf("backup-%s-%s", disk->dst,
+  
qemuBlockStorageSourceGetEffectiveNodename(disk->src));

 if (!(job = qemuBlockJobDataNew(QEMU_BLOCKJOB_TYPE_BACKUP, jobname)))
 return NULL;
-- 
2.41.0

[PATCH 19/31] qemu: backup: Use format nodename accessors

[Peter Krempa]

Both modified cases in this patch require the effective nodename as they
deal with the data being backed up.

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

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index e4db967e2c..857709b17e 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -335,9 +335,9 @@ qemuBackupDiskPrepareDataOnePush(virJSONValue *actions,
 syncmode = QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_INCREMENTAL;

 if (qemuMonitorTransactionBackup(actions,
- dd->domdisk->src->nodeformat,
+ 
qemuBlockStorageSourceGetEffectiveNodename(dd->domdisk->src),
  dd->blockjob->name,
- dd->store->nodeformat,
+ 
qemuBlockStorageSourceGetEffectiveNodename(dd->store),
  dd->incrementalBitmap,
  syncmode) < 0)
 return -1;
@@ -355,9 +355,9 @@ qemuBackupDiskPrepareDataOnePull(virJSONValue *actions,
 dd->backupdisk->exportbitmap = g_strdup(dd->incrementalBitmap);

 if (qemuMonitorTransactionBackup(actions,
- dd->domdisk->src->nodeformat,
+ 
qemuBlockStorageSourceGetEffectiveNodename(dd->domdisk->src),
  dd->blockjob->name,
- dd->store->nodeformat,
+ 
qemuBlockStorageSourceGetEffectiveNodename(dd->store),
  NULL,
  
QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_NONE) < 0)
 return -1;
-- 
2.41.0

[PATCH 25/31] qemu: driver: Convert disk stats code to use 'format' nodename accessors

[Peter Krempa]

I case of statistics we're interested in the statistics of the effective
bitmap whatever it happens to be.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6d95711cbc..2d1d0bb3e2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9368,7 +9368,7 @@ qemuDomainBlocksStatsGather(virDomainObj *vm,

 /* capacity are reported only per node-name so we need to transfer 
them */
 if (disk && disk->src &&
-(capstats = virHashLookup(blockstats, disk->src->nodeformat))) {
+(capstats = virHashLookup(blockstats, 
qemuBlockStorageSourceGetEffectiveNodename(disk->src {
 (*retstats)->capacity = capstats->capacity;
 (*retstats)->physical = capstats->physical;
 (*retstats)->wr_highest_offset = capstats->wr_highest_offset;
@@ -17348,7 +17348,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef 
*disk,

 if (QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName) {
 frontendalias = QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName;
-backendalias = n->nodeformat;
+backendalias = qemuBlockStorageSourceGetEffectiveNodename(n);
 backendstoragealias = qemuBlockStorageSourceGetStorageNodename(n);
 } else {
 /* alias may be NULL if the VM is not running */
@@ -17402,7 +17402,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef 
*disk,
 return -1;

 if (qemuDomainGetStatsOneBlock(driver, cfg, dom, params,
-   disk->mirror->nodeformat,
+   
qemuBlockStorageSourceGetEffectiveNodename(disk->mirror),
disk->mirror,
*recordnr,
stats) < 0)
@@ -17431,7 +17431,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDef 
*disk,
 return -1;

 if (qemuDomainGetStatsOneBlock(driver, cfg, dom, params,
-   
backupdisk->store->nodeformat,
+   
qemuBlockStorageSourceGetEffectiveNodename(backupdisk->store),
backupdisk->store,
*recordnr,
stats) < 0)
-- 
2.41.0

[PATCH 24/31] qemu: Convert disk backend setup code to use 'format' nodename accessors

[Peter Krempa]

The disk backend setup code is concerned only about the effective
nodename. Doing this conversion will also simplify further changes
needed to drop the 'raw' layer in cases when it's not really needed.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c  | 6 +++---
 src/qemu/qemu_domain.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 852028f014..4c1a711dd3 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1980,7 +1980,7 @@ qemuBlockStorageGetCopyOnReadProps(virDomainDiskDef *disk)
 ignore_value(virJSONValueObjectAdd(&ret,
"s:driver", "copy-on-read",
"s:node-name", priv->nodeCopyOnRead,
-   "s:file", disk->src->nodeformat,
+   "s:file", 
qemuBlockStorageSourceGetEffectiveNodename(disk->src),
"s:discard", "unmap",
NULL));

@@ -2735,10 +2735,10 @@ qemuBlockStorageSourceCreateDetectSize(GHashTable 
*blockNamedNodeData,
 {
 qemuBlockNamedNodeData *entry;

-if (!(entry = virHashLookup(blockNamedNodeData, templ->nodeformat))) {
+if (!(entry = virHashLookup(blockNamedNodeData, 
qemuBlockStorageSourceGetEffectiveNodename(templ {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to update capacity data for block node 
'%1$s'"),
-   templ->nodeformat);
+   qemuBlockStorageSourceGetEffectiveNodename(templ));
 return -1;
 }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d8d3a17e55..995aa3f79c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7891,7 +7891,7 @@ qemuDomainDiskGetTopNodename(virDomainDiskDef *disk)
 if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON)
 return priv->nodeCopyOnRead;

-return disk->src->nodeformat;
+return qemuBlockStorageSourceGetEffectiveNodename(disk->src);
 }


-- 
2.41.0

[PATCH 31/31] conf: Rename 'nodeformat' field of virStorageSource to 'nodenameformat'

[Peter Krempa]

While the name itself doesn't matter, this rename is done to prove that
all places using 'nodeformat' were converted to the appropriate
accessors.

Signed-off-by: Peter Krempa 
---
 src/conf/storage_source_conf.c | 4 ++--
 src/conf/storage_source_conf.h | 2 +-
 src/qemu/qemu_block.c  | 8 
 src/qemu/qemu_blockjob.c   | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index ea171f9945..f974a521b1 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -828,7 +828,7 @@ virStorageSourceCopy(const virStorageSource *src,
 def->backingStoreRawFormat = src->backingStoreRawFormat;
 def->snapshot = g_strdup(src->snapshot);
 def->configFile = g_strdup(src->configFile);
-def->nodeformat = g_strdup(src->nodeformat);
+def->nodenameformat = g_strdup(src->nodenameformat);
 def->nodenamestorage = g_strdup(src->nodenamestorage);
 def->compat = g_strdup(src->compat);
 def->tlsAlias = g_strdup(src->tlsAlias);
@@ -1169,7 +1169,7 @@ virStorageSourceClear(virStorageSource *def)
 virObjectUnref(def->privateData);

 VIR_FREE(def->nodenamestorage);
-VIR_FREE(def->nodeformat);
+VIR_FREE(def->nodenameformat);

 virStorageSourceBackingStoreClear(def);

diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index e9525c8f65..5a9b03b610 100644
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -370,7 +370,7 @@ struct _virStorageSource {
 virStorageFileFormat backingStoreRawFormat;

 /* metadata that allows identifying given storage source */
-char *nodeformat;  /* name of the format handler object */
+char *nodenameformat;  /* name of the format handler object */
 char *nodenamestorage; /* name of the storage object */

 /* An optional setting to enable usage of TLS for the storage source */
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index fa31028db3..0d252552de 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -81,8 +81,8 @@ void
 qemuBlockStorageSourceSetFormatNodename(virStorageSource *src,
 char *nodename)
 {
-g_free(src->nodeformat);
-src->nodeformat = nodename;
+g_free(src->nodenameformat);
+src->nodenameformat = nodename;
 }


@@ -96,7 +96,7 @@ qemuBlockStorageSourceSetFormatNodename(virStorageSource *src,
 const char *
 qemuBlockStorageSourceGetEffectiveNodename(virStorageSource *src)
 {
-return src->nodeformat;
+return src->nodenameformat;
 }


@@ -141,7 +141,7 @@ qemuBlockStorageSourceGetStorageNodename(virStorageSource 
*src)
 const char *
 qemuBlockStorageSourceGetFormatNodename(virStorageSource *src)
 {
-return src->nodeformat;
+return src->nodenameformat;
 }


diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 0913a224e3..4b5b63d287 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -617,7 +617,7 @@ qemuBlockJobCleanStorageSourceRuntime(virStorageSource *src)
 VIR_FREE(src->relPath);
 VIR_FREE(src->backingStoreRaw);
 VIR_FREE(src->nodenamestorage);
-VIR_FREE(src->nodeformat);
+VIR_FREE(src->nodenameformat);
 VIR_FREE(src->tlsAlias);
 VIR_FREE(src->tlsCertdir);
 }
-- 
2.41.0

[PATCH 07/31] qemu: block: Use proper accessors for image formatting/creation code

[Peter Krempa]

Use 'qemuBlockStorageSourceGetEffectiveStorageNodename' in all the JSON
props formatters for setting up a 'blockdev-create' job of a format
layer.

In case of the blockjob name designator we're okay to use just the
storage layer nodename as that serves only to find the appropriate
entry.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c| 10 +-
 src/qemu/qemu_blockjob.c |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0c9460f678..a98caa330e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2078,7 +2078,7 @@ 
qemuBlockStorageSourceCreateGetFormatPropsGeneric(virStorageSource *src,

 if (virJSONValueObjectAdd(&props,
   "s:driver", driver,
-  "s:file", src->nodestorage,
+  "s:file", 
qemuBlockStorageSourceGetEffectiveStorageNodename(src),
   "U:size", src->capacity,
   NULL) < 0)
 return -1;
@@ -2143,7 +2143,7 @@ 
qemuBlockStorageSourceCreateGetFormatPropsLUKS(virStorageSource *src,

 if (virJSONValueObjectAdd(&luksprops,
   "s:driver", "luks",
-  "s:file", src->nodestorage,
+  "s:file", 
qemuBlockStorageSourceGetEffectiveStorageNodename(src),
   "U:size", src->capacity,
   NULL) < 0)
 return -1;
@@ -2200,7 +2200,7 @@ 
qemuBlockStorageSourceCreateGetFormatPropsQcow2(virStorageSource *src,

 if (virJSONValueObjectAdd(&qcow2props,
   "s:driver", "qcow2",
-  "s:file", src->nodestorage,
+  "s:file", 
qemuBlockStorageSourceGetEffectiveStorageNodename(src),
   "U:size", src->capacity,
   "S:version", qcow2version,
   "P:cluster-size", src->clusterSize,
@@ -2226,7 +2226,7 @@ 
qemuBlockStorageSourceCreateGetFormatPropsQcow(virStorageSource *src,

 if (virJSONValueObjectAdd(&qcowprops,
   "s:driver", "qcow",
-  "s:file", src->nodestorage,
+  "s:file", 
qemuBlockStorageSourceGetEffectiveStorageNodename(src),
   "U:size", src->capacity,
   NULL) < 0)
 return -1;
@@ -2249,7 +2249,7 @@ 
qemuBlockStorageSourceCreateGetFormatPropsQed(virStorageSource *src,

 if (virJSONValueObjectAdd(&qedprops,
   "s:driver", "qed",
-  "s:file", src->nodestorage,
+  "s:file", 
qemuBlockStorageSourceGetEffectiveStorageNodename(src),
   "U:size", src->capacity,
   NULL) < 0)
 return -1;
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index f1d22df59f..25ac74d6c4 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -312,7 +312,7 @@ qemuBlockJobNewCreate(virDomainObj *vm,
 const char *nodename = src->nodeformat;

 if (storage)
-nodename = src->nodestorage;
+nodename = qemuBlockStorageSourceGetStorageNodename(src);

 jobname = g_strdup_printf("create-%s", nodename);

-- 
2.41.0

[PATCH 29/31] qemu: migration: Use 'format' nodename accessors in dirty bitmap migration

[Peter Krempa]

The persistent bitmaps are stored in the format layer, using 'effective'
bitmap name is the most reasonable approach in this case.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c| 11 ++-
 src/qemu/qemu_migration_cookie.c |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 949ef6d6d5..76da981d08 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2393,7 +2393,8 @@ 
qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(qemuMigrationCookie *mig,
 qemuMigrationBlockDirtyBitmapsDisk *disk;
 GSList *bitmaps = NULL;
 virDomainDiskDef *diskdef = vm->def->disks[i];
-qemuBlockNamedNodeData *nodedata = virHashLookup(blockNamedNodeData, 
diskdef->src->nodeformat);
+qemuBlockNamedNodeData *nodedata = virHashLookup(blockNamedNodeData,
+ 
qemuBlockStorageSourceGetEffectiveNodename(diskdef->src));
 size_t j;

 if (!nodedata)
@@ -4456,7 +4457,7 @@ 
qemuMigrationSrcRunPrepareBlockDirtyBitmapsMerge(virDomainObj *vm,
 granularity = b->granularity;

 if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(merge,
- 
n->nodeformat,
+ 
qemuBlockStorageSourceGetEffectiveNodename(n),
  b->name) 
< 0)
 return -1;
 }
@@ -4465,19 +4466,19 @@ 
qemuMigrationSrcRunPrepareBlockDirtyBitmapsMerge(virDomainObj *vm,
 bitmap->persistent = VIR_TRISTATE_BOOL_YES;

 if (qemuMonitorTransactionBitmapAdd(actions,
-disk->disk->src->nodeformat,
+
qemuBlockStorageSourceGetEffectiveNodename(disk->disk->src),
 bitmap->sourcebitmap,
 false, false, granularity) < 0)
 return -1;

 if (qemuMonitorTransactionBitmapMerge(actions,
-  disk->disk->src->nodeformat,
+  
qemuBlockStorageSourceGetEffectiveNodename(disk->disk->src),
   bitmap->sourcebitmap,
   &merge) < 0)
 return -1;

 tmpbmp = g_new0(qemuDomainJobPrivateMigrateTempBitmap, 1);
-tmpbmp->nodename = g_strdup(disk->disk->src->nodeformat);
+tmpbmp->nodename = 
g_strdup(qemuBlockStorageSourceGetEffectiveNodename(disk->disk->src));
 tmpbmp->bitmapname = g_strdup(bitmap->sourcebitmap);
 tmpbitmaps = g_slist_prepend(tmpbitmaps, tmpbmp);
 }
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index ba146960d5..5505fdaf22 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -1573,7 +1573,7 @@ 
qemuMigrationCookieBlockDirtyBitmapsMatchDisks(virDomainDef *def,
 return -1;
 }

-disk->nodename = disk->disk->src->nodeformat;
+disk->nodename = 
qemuBlockStorageSourceGetEffectiveNodename(disk->disk->src);
 }

 return 0;
-- 
2.41.0

[PATCH 18/31] qemuBlockStorageSourceGetFormatProps: Use new frontend name accessor

[Peter Krempa]

---
 src/qemu/qemu_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 10c2c0104b..0fa5b6e55d 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1418,7 +1418,7 @@ qemuBlockStorageSourceGetFormatProps(virStorageSource 
*src,
 src->format >= VIR_STORAGE_FILE_BACKING) {
 if (virStorageSourceIsBacking(backingStore)) {
 backingFormatterStr = "s:backing";
-backingNodename = backingStore->nodeformat;
+backingNodename = 
qemuBlockStorageSourceGetEffectiveNodename(backingStore);
 } else {
 /* chain is terminated, indicate that no detection should happen 
in qemu */
 backingFormatterStr = "n:backing";
-- 
2.41.0

[PATCH 28/31] qemu: Convert migration setup code to use 'format' layer node name accessors

[Peter Krempa]

The blockjob, NBD export and setup of the cookie data all care about the
effective nodename.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c| 6 +++---
 src/qemu/qemu_migration.c| 4 ++--
 src/qemu/qemu_migration_cookie.c | 3 ++-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index edc8edcb70..fa31028db3 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -3406,11 +3406,11 @@ qemuBlockExportAddNBD(virDomainObj *vm,
 const char *bitmaps[2] = { bitmap, NULL };

 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_EXPORT_ADD))
-return qemuMonitorNBDServerAdd(priv->mon, src->nodeformat,
+return qemuMonitorNBDServerAdd(priv->mon, 
qemuBlockStorageSourceGetEffectiveNodename(src),
exportname, writable, bitmap);

-if (!(nbdprops = qemuBlockExportGetNBDProps(src->nodeformat, exportname,
-writable, bitmaps)))
+if (!(nbdprops = 
qemuBlockExportGetNBDProps(qemuBlockStorageSourceGetEffectiveNodename(src),
+exportname, writable, 
bitmaps)))
 return -1;

 return qemuMonitorBlockExportAdd(priv->mon, &nbdprops);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 7d08df1fc5..949ef6d6d5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1012,7 +1012,7 @@ 
qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDef *disk,
 copysrc->tlsHostname = g_strdup(tlsHostname);

 qemuBlockStorageSourceSetStorageNodename(copysrc, 
g_strdup_printf("migration-%s-storage", disk->dst));
-copysrc->nodeformat = g_strdup_printf("migration-%s-format", disk->dst);
+qemuBlockStorageSourceSetFormatNodename(copysrc, 
g_strdup_printf("migration-%s-format", disk->dst));

 return g_steal_pointer(©src);
 }
@@ -1060,7 +1060,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virDomainObj *vm,
 if (mon_ret == 0)
 mon_ret = qemuMonitorBlockdevMirror(qemuDomainGetMonitor(vm), 
diskAlias, true,
 qemuDomainDiskGetTopNodename(disk),
-copysrc->nodeformat,
+
qemuBlockStorageSourceGetEffectiveNodename(copysrc),
 mirror_speed, 0, 0, mirror_shallow,
 syncWrites);

diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 7f0b7a3412..ba146960d5 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -31,6 +31,7 @@
 #include "qemu_domain.h"
 #include "qemu_migration_cookie.h"
 #include "qemu_migration_params.h"
+#include "qemu_block.h"


 #define VIR_FROM_THIS VIR_FROM_QEMU
@@ -507,7 +508,7 @@ qemuMigrationCookieAddNBD(qemuMigrationCookie *mig,
 virDomainDiskDef *disk = vm->def->disks[i];
 qemuBlockStats *entry;

-if (!(entry = virHashLookup(stats, disk->src->nodeformat)))
+if (!(entry = virHashLookup(stats, 
qemuBlockStorageSourceGetEffectiveNodename(disk->src
 continue;

 mig->nbd->disks[mig->nbd->ndisks].target = g_strdup(disk->dst);
-- 
2.41.0

[PATCH 27/31] qemu: command: Use 'format' nodename accessors for 'pflash' backend setup

[Peter Krempa]

The frontend device needs to access the blocks directly so it cares
about the effective nodename.

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

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 40de712c61..d40d3a4e13 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7032,9 +7032,11 @@ qemuBuildMachineCommandLine(virCommand *cmd,

 if (virDomainDefHasOldStyleUEFI(def)) {
 if (priv->pflash0)
-virBufferAsprintf(&buf, ",pflash0=%s", priv->pflash0->nodeformat);
+virBufferAsprintf(&buf, ",pflash0=%s",
+  
qemuBlockStorageSourceGetEffectiveNodename(priv->pflash0));
 if (def->os.loader->nvram)
-virBufferAsprintf(&buf, ",pflash1=%s", 
def->os.loader->nvram->nodeformat);
+virBufferAsprintf(&buf, ",pflash1=%s",
+  
qemuBlockStorageSourceGetEffectiveNodename(def->os.loader->nvram));
 }

 if (virDomainNumaHasHMAT(def->numa))
-- 
2.41.0

[PATCH 30/31] qemu: driver: Use 'format' nodename accessors for disk resize

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b286d94ca1..43d96739d5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9252,7 +9252,7 @@ qemuDomainBlockResize(virDomainPtr dom,
 }

 if (!qemuDiskBusIsSD(disk->bus)) {
-nodename = disk->src->nodeformat;
+nodename = qemuBlockStorageSourceGetEffectiveNodename(disk->src);
 } else {
 if (!(device = qemuAliasDiskDriveFromDisk(disk)))
 goto endjob;
-- 
2.41.0

[PATCH 22/31] qemu: domain: Use 'format' layer node name accessors for nodename setup code

[Peter Krempa]

The code setting the nodenames needs to use the 'true' nodename of the
format layer.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1b2f09a316..d8d3a17e55 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2001,7 +2001,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
 xmlNodePtr nbdkitnode = NULL;

 qemuBlockStorageSourceSetStorageNodename(src, 
virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt));
-src->nodeformat = 
virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt);
+qemuBlockStorageSourceSetFormatNodename(src, 
virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt));
 src->tlsAlias = virXPathString("string(./objects/TLSx509/@alias)", ctxt);

 if (src->sliceStorage)
@@ -2112,7 +2112,7 @@ qemuStorageSourcePrivateDataFormat(virStorageSource *src,
 g_auto(virBuffer) fdsetsChildBuf = VIR_BUFFER_INIT_CHILD(buf);

 virBufferEscapeString(&nodenamesChildBuf, "\n", qemuBlockStorageSourceGetStorageNodename(src));
-virBufferEscapeString(&nodenamesChildBuf, "\n", src->nodeformat);
+virBufferEscapeString(&nodenamesChildBuf, "\n", qemuBlockStorageSourceGetFormatNodename(src));

 if (src->sliceStorage)
 virBufferEscapeString(&nodenamesChildBuf, "\n",
@@ -2817,8 +2817,9 @@ qemuDomainVirStorageSourceFindByNodeName(virStorageSource 
*top,

 for (tmp = top; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) {
 const char *nodestorage = 
qemuBlockStorageSourceGetStorageNodename(tmp);
+const char *nodeformat = qemuBlockStorageSourceGetFormatNodename(tmp);

-if ((tmp->nodeformat && STREQ(tmp->nodeformat, nodeName)) ||
+if ((nodeformat && STREQ(nodeformat, nodeName)) ||
 (nodestorage && STREQ(nodestorage, nodeName)))
 return tmp;
 }
@@ -11144,10 +11145,11 @@ 
qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
virQEMUDriverConfig *cfg)
 {
 char *nodestorage = g_strdup_printf("%s-storage", nodenameprefix);
+char *nodeformat = g_strdup_printf("%s-format", nodenameprefix);

 /* qemuBlockStorageSourceSetStorageNodename steals 'nodestorage' */
 qemuBlockStorageSourceSetStorageNodename(src, nodestorage);
-src->nodeformat = g_strdup_printf("%s-format", nodenameprefix);
+qemuBlockStorageSourceSetFormatNodename(src, nodeformat);

 if (qemuBlockStorageSourceNeedsStorageSliceLayer(src))
 src->sliceStorage->nodename = g_strdup_printf("libvirt-%u-slice-sto", 
src->id);
@@ -11161,8 +11163,7 @@ 
qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
 qemuDomainPrepareStorageSourceConfig(src, cfg);
 qemuDomainPrepareDiskSourceData(disk, src);

-if (qemuDomainSecretStorageSourcePrepareEncryption(priv, src,
-   src->nodeformat) < 0)
+if (qemuDomainSecretStorageSourcePrepareEncryption(priv, src, nodeformat) 
< 0)
 return -1;

 if (!qemuDomainPrepareStorageSourceNbdkit(src, cfg, nodestorage, priv)) {
@@ -11698,7 +11699,7 @@ qemuDomainInitializePflashStorageSource(virDomainObj 
*vm,
 pflash0->path = g_strdup(def->os.loader->path);
 pflash0->readonly = false;
 virTristateBoolToBool(def->os.loader->readonly, &pflash0->readonly);
-pflash0->nodeformat = g_strdup("libvirt-pflash0-format");
+qemuBlockStorageSourceSetFormatNodename(pflash0, 
g_strdup("libvirt-pflash0-format"));
 qemuBlockStorageSourceSetStorageNodename(pflash0, 
g_strdup("libvirt-pflash0-storage"));

 if (def->os.loader->nvram) {
-- 
2.41.0

[PATCH 10/31] qemu: domain: Rework assignment of 'storage' nodenames to use new accessors

[Peter Krempa]

Refactor the code which assigns the 'storage' layer nodenames for disks.
scsi hostdevs and pflash backend.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c| 25 ++---
 src/qemu/qemu_migration.c |  2 +-
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 78dc99d243..885c5ee96b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11143,7 +11143,10 @@ 
qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
qemuDomainObjPrivate *priv,
virQEMUDriverConfig *cfg)
 {
-src->nodestorage = g_strdup_printf("%s-storage", nodenameprefix);
+char *nodestorage = g_strdup_printf("%s-storage", nodenameprefix);
+
+/* qemuBlockStorageSourceSetStorageNodename steals 'nodestorage' */
+qemuBlockStorageSourceSetStorageNodename(src, nodestorage);
 src->nodeformat = g_strdup_printf("%s-format", nodenameprefix);

 if (qemuBlockStorageSourceNeedsStorageSliceLayer(src))
@@ -11162,18 +11165,17 @@ 
qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
src->nodeformat) < 0)
 return -1;

-if (!qemuDomainPrepareStorageSourceNbdkit(src, cfg, src->nodestorage, 
priv)) {
+if (!qemuDomainPrepareStorageSourceNbdkit(src, cfg, nodestorage, priv)) {
 /* If we're using nbdkit to serve the storage source, we don't pass
  * authentication secrets to qemu, but will pass them to nbdkit 
instead */
-if (qemuDomainSecretStorageSourcePrepareAuth(priv, src,
- src->nodestorage) < 0)
+if (qemuDomainSecretStorageSourcePrepareAuth(priv, src, nodestorage) < 
0)
 return -1;
 }

-if (qemuDomainPrepareStorageSourcePR(src, priv, src->nodestorage) < 0)
+if (qemuDomainPrepareStorageSourcePR(src, priv, nodestorage) < 0)
 return -1;

-if (qemuDomainPrepareStorageSourceTLS(src, cfg, src->nodestorage,
+if (qemuDomainPrepareStorageSourceTLS(src, cfg, nodestorage,
   priv) < 0)
 return -1;

@@ -11297,12 +11299,14 @@ qemuDomainPrepareHostdevSCSI(virDomainHostdevDef 
*hostdev,
 }

 if (src) {
-const char *backendalias = hostdev->info->alias;
+char *backendalias;

 src->readonly = hostdev->readonly;
 src->id = qemuDomainStorageIDNew(priv);
-src->nodestorage = g_strdup_printf("libvirt-%d-backend", src->id);
-backendalias = src->nodestorage;
+backendalias = g_strdup_printf("libvirt-%d-backend", src->id);
+
+/* 'src' takes ownership of 'backendalias' */
+qemuBlockStorageSourceSetStorageNodename(src, backendalias);

 if (src->auth) {
 virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
@@ -11695,8 +11699,7 @@ qemuDomainInitializePflashStorageSource(virDomainObj 
*vm,
 pflash0->readonly = false;
 virTristateBoolToBool(def->os.loader->readonly, &pflash0->readonly);
 pflash0->nodeformat = g_strdup("libvirt-pflash0-format");
-pflash0->nodestorage = g_strdup("libvirt-pflash0-storage");
-
+qemuBlockStorageSourceSetStorageNodename(pflash0, 
g_strdup("libvirt-pflash0-storage"));

 if (def->os.loader->nvram) {
 if (qemuDomainPrepareStorageSourceBlockdevNodename(NULL,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 011482c2b5..7d08df1fc5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1011,7 +1011,7 @@ 
qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDef *disk,
 copysrc->tlsAlias = g_strdup(tlsAlias);
 copysrc->tlsHostname = g_strdup(tlsHostname);

-copysrc->nodestorage = g_strdup_printf("migration-%s-storage", disk->dst);
+qemuBlockStorageSourceSetStorageNodename(copysrc, 
g_strdup_printf("migration-%s-storage", disk->dst));
 copysrc->nodeformat = g_strdup_printf("migration-%s-format", disk->dst);

 return g_steal_pointer(©src);
-- 
2.41.0

[PATCH 05/31] tests: Use 'storage' layer nodename accessors in tests

[Peter Krempa]

Convert all places in tests to use the 'storage' layer nodename
accessors instead of (virStorageSource)->nodestorage.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c   | 4 ++--
 tests/qemumonitorjsontest.c | 2 +-
 tests/qemuxml2argvtest.c| 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index edfe7719c8..9a72a67ce2 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -233,7 +233,7 @@ testQemuDiskXMLToJSONFakeSecrets(virStorageSource *src)

 srcpriv->secinfo->username = g_strdup(src->auth->username);
 srcpriv->secinfo->alias = g_strdup_printf("%s-secalias",
-  NULLSTR(src->nodestorage));
+  
NULLSTR(qemuBlockStorageSourceGetStorageNodename(src)));
 }

 if (src->encryption) {
@@ -667,7 +667,7 @@ testQemuBackupIncrementalBitmapCalculateGetFakeImage(size_t 
idx)
ret->type = VIR_STORAGE_TYPE_FILE;
ret->format = VIR_STORAGE_FILE_QCOW2;
ret->path = g_strdup_printf("/image%zu", idx);
-   ret->nodestorage = g_strdup_printf("libvirt-%zu-storage", idx);
+   qemuBlockStorageSourceSetStorageNodename(ret, 
g_strdup_printf("libvirt-%zu-storage", idx));
ret->nodeformat = g_strdup_printf("libvirt-%zu-format", idx);

return ret;
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 2e7b661db4..6b82e3841b 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2601,7 +2601,7 @@ testQemuMonitorJSONBlockdevReopen(const void *opaque)
 src->format = VIR_STORAGE_FILE_QCOW2;
 src->readonly = true;
 src->nodeformat = g_strdup("test node");
-src->nodestorage = g_strdup("backing nodename");
+qemuBlockStorageSourceSetStorageNodename(src, g_strdup("backing 
nodename"));
 src->backingStore = virStorageSourceNew();

 if (qemuMonitorTestAddItem(test, "blockdev-reopen", "{\"return\":{}}") < 0)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a454dcb205..48058cb924 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -12,6 +12,7 @@
 # include "internal.h"
 # include "viralloc.h"
 # include "viridentity.h"
+# include "qemu/qemu_block.h"
 # include "qemu/qemu_capabilities.h"
 # include "qemu/qemu_domain.h"
 # include "qemu/qemu_migration.h"
@@ -338,7 +339,7 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv,

 srcpriv = qemuDomainStorageSourcePrivateFetch(src);

-srcpriv->fdpass = qemuFDPassNew(src->nodestorage, priv);
+srcpriv->fdpass = 
qemuFDPassNew(qemuBlockStorageSourceGetStorageNodename(src), priv);
 qemuFDPassAddFD(srcpriv->fdpass, &fd, "-vdpa");
 }
 }
-- 
2.41.0

[PATCH 15/31] conf: Rename 'nodestorage' field of virStorageSource to 'nodenamestorage'

[Peter Krempa]

While the name itself doesn't matter, this rename is done to prove that
all places using 'nodestorage' were converted to the appropriate
accessors.

Signed-off-by: Peter Krempa 
---
 src/conf/storage_source_conf.c | 4 ++--
 src/conf/storage_source_conf.h | 2 +-
 src/qemu/qemu_block.c  | 8 
 src/qemu/qemu_blockjob.c   | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index 61a433ab1f..ea171f9945 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -829,7 +829,7 @@ virStorageSourceCopy(const virStorageSource *src,
 def->snapshot = g_strdup(src->snapshot);
 def->configFile = g_strdup(src->configFile);
 def->nodeformat = g_strdup(src->nodeformat);
-def->nodestorage = g_strdup(src->nodestorage);
+def->nodenamestorage = g_strdup(src->nodenamestorage);
 def->compat = g_strdup(src->compat);
 def->tlsAlias = g_strdup(src->tlsAlias);
 def->tlsCertdir = g_strdup(src->tlsCertdir);
@@ -1168,7 +1168,7 @@ virStorageSourceClear(virStorageSource *def)
 virStorageAuthDefFree(def->auth);
 virObjectUnref(def->privateData);

-VIR_FREE(def->nodestorage);
+VIR_FREE(def->nodenamestorage);
 VIR_FREE(def->nodeformat);

 virStorageSourceBackingStoreClear(def);
diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index fc6c67f426..e9525c8f65 100644
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -371,7 +371,7 @@ struct _virStorageSource {

 /* metadata that allows identifying given storage source */
 char *nodeformat;  /* name of the format handler object */
-char *nodestorage; /* name of the storage object */
+char *nodenamestorage; /* name of the storage object */

 /* An optional setting to enable usage of TLS for the storage source */
 virTristateBool haveTLS;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 7355cb0b5e..a2414dc2e3 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -64,8 +64,8 @@ void
 qemuBlockStorageSourceSetStorageNodename(virStorageSource *src,
  char *nodename)
 {
-g_free(src->nodestorage);
-src->nodestorage = nodename;
+g_free(src->nodenamestorage);
+src->nodenamestorage = nodename;
 }


@@ -83,7 +83,7 @@ 
qemuBlockStorageSourceGetEffectiveStorageNodename(virStorageSource *src)
 src->sliceStorage->nodename)
 return src->sliceStorage->nodename;

-return src->nodestorage;
+return src->nodenamestorage;
 }


@@ -96,7 +96,7 @@ 
qemuBlockStorageSourceGetEffectiveStorageNodename(virStorageSource *src)
 const char *
 qemuBlockStorageSourceGetStorageNodename(virStorageSource *src)
 {
-return src->nodestorage;
+return src->nodenamestorage;
 }


diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 25ac74d6c4..12f3d59df5 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -612,7 +612,7 @@ qemuBlockJobCleanStorageSourceRuntime(virStorageSource *src)
 src->detected = false;
 VIR_FREE(src->relPath);
 VIR_FREE(src->backingStoreRaw);
-VIR_FREE(src->nodestorage);
+VIR_FREE(src->nodenamestorage);
 VIR_FREE(src->nodeformat);
 VIR_FREE(src->tlsAlias);
 VIR_FREE(src->tlsCertdir);
-- 
2.41.0

[PATCH 09/31] qemu: block: Convert disk 'storage' backend JSON props generator to new accessors

[Peter Krempa]

We need to use the 'effective' storage nodename (one which includes the
optional storage slice 'raw' intermediate layer) in the code which
formats the 'format' layer props.

All other cases need the real storage driver nodename as they either
generate the 'storage' layer props, or the storage slice, which refers
to the proper storage backend.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index a98caa330e..1fc36569a9 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1060,8 +1060,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSource 
*src,
 return NULL;

 if (!onlytarget) {
-if (qemuBlockNodeNameValidate(src->nodestorage) < 0 ||
-virJSONValueObjectAdd(&fileprops, "S:node-name", src->nodestorage, 
NULL) < 0)
+if 
(qemuBlockNodeNameValidate(qemuBlockStorageSourceGetStorageNodename(src)) < 0 ||
+virJSONValueObjectAdd(&fileprops,
+  "S:node-name", 
qemuBlockStorageSourceGetStorageNodename(src),
+  NULL) < 0)
 return NULL;

 if (!legacy) {
@@ -1358,10 +1360,6 @@ qemuBlockStorageSourceGetFormatProps(virStorageSource 
*src,
 g_autoptr(virJSONValue) props = NULL;
 const char *backingFormatterStr = NULL;
 const char *backingNodename = NULL;
-const char *storagenode = src->nodestorage;
-
-if (qemuBlockStorageSourceNeedsStorageSliceLayer(src))
-storagenode = src->sliceStorage->nodename;

 if (virStorageSourceIsBacking(backingStore) &&
 src->format < VIR_STORAGE_FILE_BACKING) {
@@ -1386,7 +1384,7 @@ qemuBlockStorageSourceGetFormatProps(virStorageSource 
*src,
 return NULL;

 if (virJSONValueObjectAdd(&props,
-  "s:file", storagenode,
+  "s:file", 
qemuBlockStorageSourceGetEffectiveStorageNodename(src),
   backingFormatterStr, backingNodename,
   NULL) < 0)
 return 0;
@@ -1408,7 +1406,7 @@ 
qemuBlockStorageSourceGetBlockdevStorageSliceProps(virStorageSource *src)
   "s:node-name", src->sliceStorage->nodename,
   "U:offset", src->sliceStorage->offset,
   "U:size", src->sliceStorage->size,
-  "s:file", src->nodestorage,
+  "s:file", 
qemuBlockStorageSourceGetStorageNodename(src),
   "b:auto-read-only", true,
   "s:discard", "unmap",
   NULL) < 0)
-- 
2.41.0

[PATCH 16/31] qemu: block: Add accessors for format layer node names

[Peter Krempa]

Introduce a set of accessors, which return node names based on
semantics. This will allow to us to modify how we setup the backing
chain in cases when e.g. the format driver can be omitted, without
breaking all the code.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 31 +++
 src/qemu/qemu_block.h |  7 +++
 2 files changed, 38 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index a2414dc2e3..cba1fb1c1e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -69,6 +69,23 @@ qemuBlockStorageSourceSetStorageNodename(virStorageSource 
*src,
 }


+/**
+ * qemuBlockStorageSourceSetFormatNodename:
+ * @src: virStorageSource to set the format nodename
+ * @nodename: The node name to set (stolen)
+ *
+ * Sets @nodename as the format node name of @src. Using NULL @nodename clears
+ * the nodename. @src takes ownership of @nodename.
+ */
+void
+qemuBlockStorageSourceSetFormatNodename(virStorageSource *src,
+char *nodename)
+{
+g_free(src->nodeformat);
+src->nodeformat = nodename;
+}
+
+
 /**
  * qemuBlockStorageSourceGetEffectiveStorageNodename:
  * @src: virStorageSource to get the effective nodename of
@@ -100,6 +117,20 @@ qemuBlockStorageSourceGetStorageNodename(virStorageSource 
*src)
 }


+/**
+ * qemuBlockStorageSourceGetFormatNodename:
+ * @src: virStorageSource to get the effective nodename of
+ *
+ * Gets the nodename corresponding to the format layer. Useful when accessing
+ * format specific features. Returns NULL if there is no format layer.
+ */
+const char *
+qemuBlockStorageSourceGetFormatNodename(virStorageSource *src)
+{
+return src->nodeformat;
+}
+
+
 /**
  * qemuBlockStorageSourceSupportsConcurrentAccess:
  * @src: disk storage source
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index ecc5711dcd..6ed0aa85b2 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -30,12 +30,19 @@ void
 qemuBlockStorageSourceSetStorageNodename(virStorageSource *src,
  char *nodename);

+void
+qemuBlockStorageSourceSetFormatNodename(virStorageSource *src,
+char *nodename);
+
 const char *
 qemuBlockStorageSourceGetEffectiveStorageNodename(virStorageSource *src);

 const char *
 qemuBlockStorageSourceGetStorageNodename(virStorageSource *src);

+const char *
+qemuBlockStorageSourceGetFormatNodename(virStorageSource *src);
+

 typedef struct qemuBlockNodeNameBackingChainData 
qemuBlockNodeNameBackingChainData;
 struct qemuBlockNodeNameBackingChainData {
-- 
2.41.0

[PATCH 12/31] qemu: Refactor storage backend 'storage' layer helepr object setup

[Peter Krempa]

Use the new nodename accessors for any storage layer helper object.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c  | 2 +-
 src/qemu/qemu_nbdkit.c  | 4 ++--
 src/qemu/qemu_process.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 885c5ee96b..1b2f09a316 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11106,7 +11106,7 @@ qemuDomainPrepareStorageSourceFDs(virStorageSource *src,

 srcpriv = qemuDomainStorageSourcePrivateFetch(src);

-srcpriv->fdpass = qemuFDPassNew(src->nodestorage, priv);
+srcpriv->fdpass = 
qemuFDPassNew(qemuBlockStorageSourceGetStorageNodename(src), priv);

 for (i = 0; i < fdt->nfds; i++) {
 g_autofree char *idx = g_strdup_printf("%zu", i);
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 3ad63cfaa0..85e61be44c 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -923,7 +923,7 @@ qemuNbdkitStopStorageSource(virStorageSource *src,

 if (priv && priv->nbdkitProcess &&
 qemuNbdkitProcessStop(priv->nbdkitProcess, vm) < 0)
-VIR_WARN("Unable to stop nbdkit for storage source '%s'", 
src->nodestorage);
+VIR_WARN("Unable to stop nbdkit for storage source '%s'", 
qemuBlockStorageSourceGetStorageNodename(src));
 }
 }

@@ -1173,7 +1173,7 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc,

 logfd = qemuLogContextGetWriteFD(logContext);

-VIR_DEBUG("starting nbdkit process for %s", proc->source->nodestorage);
+VIR_DEBUG("starting nbdkit process for %s", 
qemuBlockStorageSourceGetStorageNodename(proc->source));
 virCommandSetErrorFD(cmd, &logfd);
 virCommandSetOutputFD(cmd, &logfd);
 virCommandSetPidFile(cmd, proc->pidfile);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ae0bb7bf80..63c0c62a46 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6771,7 +6771,7 @@ qemuProcessPrepareHostStorageSourceVDPA(virStorageSource 
*src,

 srcpriv = qemuDomainStorageSourcePrivateFetch(src);

-srcpriv->fdpass = qemuFDPassNew(src->nodestorage, priv);
+srcpriv->fdpass = 
qemuFDPassNew(qemuBlockStorageSourceGetStorageNodename(src), priv);
 qemuFDPassAddFD(srcpriv->fdpass, &vdpafd, "-vdpa");
 return 0;
 }
-- 
2.41.0

[PATCH 01/31] qemu: domain: Identify blockjobs by storage nodename in VM status XML

[Peter Krempa]

Use the node name of the storage access driver to identify the block job
volumes. This will prepare the blockjob code to the possibility that the
format layer may be missing. Our lookup code can find either of them,
thus we can safely switch.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c   |  8 
 tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 12 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index eec7bed28b..918b5a14e1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2288,13 +2288,13 @@ qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobData 
*job,
 g_auto(virBuffer) disabledBitmapsBuf = VIR_BUFFER_INIT_CHILD(buf);

 if (job->data.commit.base)
-virBufferAsprintf(buf, "\n", 
job->data.commit.base->nodeformat);
+virBufferAsprintf(buf, "\n", 
job->data.commit.base->nodestorage);

 if (job->data.commit.top)
-virBufferAsprintf(buf, "\n", 
job->data.commit.top->nodeformat);
+virBufferAsprintf(buf, "\n", 
job->data.commit.top->nodestorage);

 if (job->data.commit.topparent)
-virBufferAsprintf(buf, "\n", 
job->data.commit.topparent->nodeformat);
+virBufferAsprintf(buf, "\n", 
job->data.commit.topparent->nodestorage);

 if (job->data.commit.deleteCommittedImages)
 virBufferAddLit(buf, "\n");
@@ -2357,7 +2357,7 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void 
*payload,
 switch ((qemuBlockJobType) job->type) {
 case QEMU_BLOCKJOB_TYPE_PULL:
 if (job->data.pull.base)
-virBufferAsprintf(&childBuf, "\n", 
job->data.pull.base->nodeformat);
+virBufferAsprintf(&childBuf, "\n", 
job->data.pull.base->nodestorage);
 break;

 case QEMU_BLOCKJOB_TYPE_COMMIT:
diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml 
b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
index b62b3149c2..380ef053d2 100644
--- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
+++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
@@ -238,14 +238,14 @@
 
 
   
-  
-  
-  
+  
+  
+  
 
 
   
-  
-  
+  
+  
   
 
 
@@ -301,7 +301,7 @@
 
 
   
-  
+  
 
 
   
-- 
2.41.0

[PATCH 04/31] qemu: block: Add accessors for protocol/storage node names

[Peter Krempa]

Introduce a set of accessors, which return node names based on
semantics. This will allow to us to modify how we setup the backing
chain in cases when e.g. the format driver can be omitted, without
breaking all the code.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 48 +++
 src/qemu/qemu_block.h | 11 ++
 2 files changed, 59 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 07bc8ede76..0c9460f678 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -52,6 +52,54 @@ qemuBlockNodeNameValidate(const char *nn)
 }


+/**
+ * qemuBlockStorageSourceSetStorageNodename:
+ * @src: virStorageSource to set the storage nodename
+ * @nodename: The node name to set (stolen)
+ *
+ * Sets @nodename as the storage node name of @src. Using NULL @nodename clears
+ * the nodename. @src takes ownership of @nodename.
+ */
+void
+qemuBlockStorageSourceSetStorageNodename(virStorageSource *src,
+ char *nodename)
+{
+g_free(src->nodestorage);
+src->nodestorage = nodename;
+}
+
+
+/**
+ * qemuBlockStorageSourceGetEffectiveStorageNodename:
+ * @src: virStorageSource to get the effective nodename of
+ *
+ * Gets the nodename that exposes the storage corresponding to @src, without
+ * the format driver applied. This function always returns a name.
+ */
+const char *
+qemuBlockStorageSourceGetEffectiveStorageNodename(virStorageSource *src)
+{
+if (src->sliceStorage &&
+src->sliceStorage->nodename)
+return src->sliceStorage->nodename;
+
+return src->nodestorage;
+}
+
+
+/**
+ * qemuBlockStorageSourceGetStorageNodename:
+ * @src: virStorageSource to get the effective nodename of
+ *
+ * Gets the nodename corresponding to the real backing storage format layer.
+ */
+const char *
+qemuBlockStorageSourceGetStorageNodename(virStorageSource *src)
+{
+return src->nodestorage;
+}
+
+
 /**
  * qemuBlockStorageSourceSupportsConcurrentAccess:
  * @src: disk storage source
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index cf5eaf87f3..ecc5711dcd 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -26,6 +26,17 @@
 #include "virjson.h"
 #include "viruri.h"

+void
+qemuBlockStorageSourceSetStorageNodename(virStorageSource *src,
+ char *nodename);
+
+const char *
+qemuBlockStorageSourceGetEffectiveStorageNodename(virStorageSource *src);
+
+const char *
+qemuBlockStorageSourceGetStorageNodename(virStorageSource *src);
+
+
 typedef struct qemuBlockNodeNameBackingChainData 
qemuBlockNodeNameBackingChainData;
 struct qemuBlockNodeNameBackingChainData {
 char *qemufilename; /* name of the image from qemu */
-- 
2.41.0

[PATCH 03/31] qemu: block: Rename qemuBlockStorageSourceGetBlockdevProps

[Peter Krempa]

Use qemuBlockStorageSourceGetFormatProps as it formats the properties of
the 'format' driver in qemu. Adjust the comment which was hinting
otherwise.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 16 +++-
 src/qemu/qemu_block.h |  4 ++--
 tests/qemublocktest.c |  2 +-
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 9b6d901e8c..07bc8ede76 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1295,18 +1295,17 @@ 
qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSource *src)


 /**
- * qemuBlockStorageSourceGetBlockdevProps:
+ * qemuBlockStorageSourceGetFormatProps:
  *
  * @src: storage source to format
  * @backingStore: a storage source to use as backing of @src
  *
- * Formats @src into a JSON object which can be used with blockdev-add or
- * -blockdev. The formatted object contains both the storage and format layer
- * in nested form including link to the backing chain layer if necessary.
+ * Formats properties of @src related to the format blockdev driver in qemu
+ * into a JSON object which can be used with blockdev-add or -blockdev.
  */
 virJSONValue *
-qemuBlockStorageSourceGetBlockdevProps(virStorageSource *src,
-   virStorageSource *backingStore)
+qemuBlockStorageSourceGetFormatProps(virStorageSource *src,
+ virStorageSource *backingStore)
 {
 g_autoptr(virJSONValue) props = NULL;
 const char *backingFormatterStr = NULL;
@@ -1434,8 +1433,7 @@ 
qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSource *src,

 data = g_new0(qemuBlockStorageSourceAttachData, 1);

-if (!(data->formatProps = qemuBlockStorageSourceGetBlockdevProps(src,
- 
backingStore)) ||
+if (!(data->formatProps = qemuBlockStorageSourceGetFormatProps(src, 
backingStore)) ||
 !(data->storageProps = qemuBlockStorageSourceGetBackendProps(src,
  
backendpropsflags)))
 return NULL;
@@ -3049,7 +3047,7 @@ qemuBlockReopenFormatMon(qemuMonitor *mon,
 g_autoptr(virJSONValue) srcprops = NULL;
 g_autoptr(virJSONValue) reopenoptions = virJSONValueNewArray();

-if (!(srcprops = qemuBlockStorageSourceGetBlockdevProps(src, 
src->backingStore)))
+if (!(srcprops = qemuBlockStorageSourceGetFormatProps(src, 
src->backingStore)))
 return -1;

 if (virJSONValueArrayAppend(reopenoptions, &srcprops) < 0)
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 530d88d28e..cf5eaf87f3 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -57,8 +57,8 @@ virURI *
 qemuBlockStorageSourceGetURI(virStorageSource *src);

 virJSONValue *
-qemuBlockStorageSourceGetBlockdevProps(virStorageSource *src,
-   virStorageSource *backingStore);
+qemuBlockStorageSourceGetFormatProps(virStorageSource *src,
+ virStorageSource *backingStore);

 virJSONValue *
 qemuBlockStorageGetCopyOnReadProps(virDomainDiskDef *disk);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 8bad69e7ac..edfe7719c8 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -301,7 +301,7 @@ testQemuDiskXMLToProps(const void *opaque)

 qemuDomainPrepareDiskSourceData(disk, n);

-if (!(formatProps = qemuBlockStorageSourceGetBlockdevProps(n, 
n->backingStore)) ||
+if (!(formatProps = qemuBlockStorageSourceGetFormatProps(n, 
n->backingStore)) ||
 !(storageSrcOnlyProps = qemuBlockStorageSourceGetBackendProps(n, 
backendpropsflagstarget)) ||
 !(storageProps = qemuBlockStorageSourceGetBackendProps(n, 
backendpropsflagsnormal)) ||
 !(backingstore = qemuBlockGetBackingStoreString(n, true))) {
-- 
2.41.0

[PATCH 06/31] qemuDomainVirStorageSourceFindByNodeName: Use proper accessor

[Peter Krempa]

The lookup by nodename requires the proper storage nodename which we use
also in status XML.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 918b5a14e1..f891defa91 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2812,8 +2812,10 @@ 
qemuDomainVirStorageSourceFindByNodeName(virStorageSource *top,
 virStorageSource *tmp;

 for (tmp = top; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) {
+const char *nodestorage = 
qemuBlockStorageSourceGetStorageNodename(tmp);
+
 if ((tmp->nodeformat && STREQ(tmp->nodeformat, nodeName)) ||
-(tmp->nodestorage && STREQ(tmp->nodestorage, nodeName)))
+(nodestorage && STREQ(nodestorage, nodeName)))
 return tmp;
 }

-- 
2.41.0

[PATCH 02/31] qemu: block: Refactor logic in qemuBlockStorageSourceGetBlockdevProps

[Peter Krempa]

Restructure the conditions so that we can use virJSONValueObjectAdd with
a clearer logic for backing store control.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 46 ++-
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 7e870baa2f..9b6d901e8c 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1309,39 +1309,41 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSource 
*src,
virStorageSource *backingStore)
 {
 g_autoptr(virJSONValue) props = NULL;
+const char *backingFormatterStr = NULL;
+const char *backingNodename = NULL;
 const char *storagenode = src->nodestorage;

 if (qemuBlockStorageSourceNeedsStorageSliceLayer(src))
 storagenode = src->sliceStorage->nodename;

-if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src)))
-return NULL;
-
-if (virJSONValueObjectAppendString(props, "file", storagenode) < 0)
+if (virStorageSourceIsBacking(backingStore) &&
+src->format < VIR_STORAGE_FILE_BACKING) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("storage format '%1$s' does not support backing 
store"),
+   virStorageFileFormatTypeToString(src->format));
 return NULL;
+}

-if (backingStore) {
-if (src->format >= VIR_STORAGE_FILE_BACKING) {
-if (virStorageSourceIsBacking(backingStore)) {
-if (virJSONValueObjectAppendString(props, "backing",
-   backingStore->nodeformat) < 
0)
-return NULL;
-} else {
-/* chain is terminated, indicate that no detection should 
happen
- * in qemu */
-if (virJSONValueObjectAppendNull(props, "backing") < 0)
-return NULL;
-}
+if (backingStore &&
+src->format >= VIR_STORAGE_FILE_BACKING) {
+if (virStorageSourceIsBacking(backingStore)) {
+backingFormatterStr = "s:backing";
+backingNodename = backingStore->nodeformat;
 } else {
-if (virStorageSourceIsBacking(backingStore)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("storage format '%1$s' does not support 
backing store"),
-   virStorageFileFormatTypeToString(src->format));
-return NULL;
-}
+/* chain is terminated, indicate that no detection should happen 
in qemu */
+backingFormatterStr = "n:backing";
 }
 }

+if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src)))
+return NULL;
+
+if (virJSONValueObjectAdd(&props,
+  "s:file", storagenode,
+  backingFormatterStr, backingNodename,
+  NULL) < 0)
+return 0;
+
 return g_steal_pointer(&props);
 }

-- 
2.41.0

[PATCH 00/31] qemu: Add accessors for 'storage' and 'format' nodenames and refactor callers ('raw' driver removal part 1)

[Peter Krempa]

This is a prequel series necessary for removing the dummy 'raw' driver
node if it isn't needed for performance reasons.

Peter Krempa (31):
  qemu: domain: Identify blockjobs by storage nodename in VM status XML
  qemu: block: Refactor logic in qemuBlockStorageSourceGetBlockdevProps
  qemu: block: Rename qemuBlockStorageSourceGetBlockdevProps
  qemu: block: Add accessors for protocol/storage node names
  tests: Use 'storage' layer nodename accessors in tests
  qemuDomainVirStorageSourceFindByNodeName: Use proper accessor
  qemu: block: Use proper accessors for image formatting/creation code
  qemu: domain: Convert the status XML code for 'storage' nodenames to
new accessors
  qemu: block: Convert disk 'storage' backend JSON props generator to
new accessors
  qemu: domain: Rework assignment of 'storage' nodenames to use new
accessors
  qemu: Refactor storage backend attach/detach setup code to use
'storage' nodename accessors
  qemu: Refactor storage backend 'storage' layer helepr object setup
  qemuDomainGetStatsBlockExportDisk: Use 'storage' node name accessors
  qemuDomainSetBlockThreshold: Use 'storage' node name accessor
  conf: Rename 'nodestorage' field of virStorageSource to
'nodenamestorage'
  qemu: block: Add accessors for format layer node names
  qemu: block: Add accessors for storage source effective nodename
  qemuBlockStorageSourceGetFormatProps: Use new frontend name accessor
  qemu: backup: Use format nodename accessors
  qemu: blockjob: Use 'format' nodename accessors for job naming
  qemu: block: Use 'format' nodename accessors in '-blockdev' setup code
  qemu: domain: Use 'format' layer node name accessors for nodename
setup code
  tests: Use 'format' layer nodename accessors in test code
  qemu: Convert disk backend setup code to use 'format' nodename
accessors
  qemu: driver: Convert disk stats code to use 'format' nodename
accessors
  qemu: Use 'format' nodename accessors for block dirty bitmap
operations
  qemu: command: Use 'format' nodename accessors for 'pflash' backend
setup
  qemu: Convert migration setup code to use 'format' layer node name
accessors
  qemu: migration: Use 'format' nodename accessors in dirty bitmap
migration
  qemu: driver: Use 'format' nodename accessors for disk resize
  conf: Rename 'nodeformat' field of virStorageSource to
'nodenameformat'

 src/conf/storage_source_conf.c|   8 +-
 src/conf/storage_source_conf.h|   4 +-
 src/qemu/qemu_backup.c|   8 +-
 src/qemu/qemu_block.c | 238 --
 src/qemu/qemu_block.h |  25 +-
 src/qemu/qemu_blockjob.c  |  24 +-
 src/qemu/qemu_checkpoint.c|   9 +-
 src/qemu/qemu_command.c   |  12 +-
 src/qemu/qemu_domain.c|  68 ++---
 src/qemu/qemu_driver.c|  25 +-
 src/qemu/qemu_migration.c |  17 +-
 src/qemu/qemu_migration_cookie.c  |   5 +-
 src/qemu/qemu_nbdkit.c|   4 +-
 src/qemu/qemu_process.c   |   2 +-
 src/qemu/qemu_snapshot.c  |   6 +-
 tests/qemublocktest.c |  16 +-
 tests/qemumonitorjsontest.c   |   4 +-
 .../blockjob-blockdev-in.xml  |  12 +-
 tests/qemuxml2argvtest.c  |   3 +-
 19 files changed, 314 insertions(+), 176 deletions(-)

-- 
2.41.0

Re: [libvirt] [PATCHv2 37/62] qemu: process: Setup disk io throttling?for -blockdev

[Peter Krempa]

On Mon, Oct 16, 2023 at 14:19:08 +, Chun Feng Wu wrote:
> Thanks for your info! Currently, the iotune in libvirt’s xml indeed works 
> well, BTW, is there any plan for such “re-engineering”?

No, there are no plans, as the current approach covers the vast majority
of use cases. Spending significant engineering resources on something
which will not get much use is not in the interest of the comunity. 

RE: [libvirt] [PATCHv2 37/62] qemu: process: Setup disk io throttling for -blockdev

[Chun Feng Wu]

Thanks for your info! Currently, the iotune in libvirt’s xml indeed works well, 
BTW, is there any plan for such “re-engineering”?

From: Peter Krempa 
Date: Monday, October 16, 2023 at 21:53
To: Chun Feng Wu 
Cc: libvir-list@redhat.com 
Subject: [EXTERNAL] Re: [libvirt] [PATCHv2 37/62] qemu: process: Setup disk io 
throttling for -blockdev
On Mon, Oct 16, 2023 at 13:36:42 +, Chun Feng Wu wrote:
> Does this libvirt throttling support “chained throttle filters” for single 
> disk described in QEMU doc: 
> https://github.com/qemu/qemu/blob/master/docs/throttle.txt  (“The 'throttle' 
> block filter”)

No, libvirt currently still uses the older approach by using
'block_set_io_throttle' QMP command. This was used at the beginning
because the 'throttle' layer was not yet ready at the point when libvirt
adopted -blockdev.

It is sufficient for the throttling options that are currently
configurable via libvirt's XML.

Note that to use the full potential of the throttle groups and
hierarchical throttling will require significant re-engineering of the
XML configuration of throttling.

Re: [libvirt] [PATCHv2 37/62] qemu: process: Setup disk io throttling for -blockdev

[Peter Krempa]

On Mon, Oct 16, 2023 at 13:36:42 +, Chun Feng Wu wrote:
> Does this libvirt throttling support “chained throttle filters” for single 
> disk described in QEMU doc: 
> https://github.com/qemu/qemu/blob/master/docs/throttle.txt (“The 'throttle' 
> block filter”)

No, libvirt currently still uses the older approach by using
'block_set_io_throttle' QMP command. This was used at the beginning
because the 'throttle' layer was not yet ready at the point when libvirt
adopted -blockdev.

It is sufficient for the throttling options that are currently
configurable via libvirt's XML.

Note that to use the full potential of the throttle groups and
hierarchical throttling will require significant re-engineering of the
XML configuration of throttling.

[libvirt] [PATCHv2 37/62] qemu: process: Setup disk io throttling for -blockdev

[Chun Feng Wu]

Does this libvirt throttling support “chained throttle filters” for single disk 
described in QEMU doc: 
https://github.com/qemu/qemu/blob/master/docs/throttle.txt (“The 'throttle' 
block filter”)

Re: [PATCH v4 03/10] migration: migrate 'inc' command option is deprecated.

[Juan Quintela]

Markus Armbruster  wrote:
> Juan Quintela  writes:
>
>> Markus Armbruster  wrote:
>>> Juan Quintela  writes:
>> So what I want, I want to remove -i/-b in the next version (9.0?).  For
>> the other, I want to remove it, but I don't care if the code is around
>> in "deprecated" state for another couple of years if there are still
>> people that feel that they want it.
>>
>> This is the reason that I put a pointer for -i/-b to
>> @block/@block-incremental.  They are "perfect" replacements.
>>
>> I can put here to use blockdev-mirror + NBD, but the replacement is not
>> so direct.
>>
>> Does this make sense?
>
> I see where you're coming from.  Now let's change perspective for a
> minute: what's the purpose of deprecating stuff?
>
> We normally deprecate with intent to remove.
>
> We don't remove right away, because we promised to first deprecate for a
> grace period, so users can adjust in an orderly manner.  The deprecation
> serves as signal "you need to adjust".  The documentation that comes
> with it should help with the adjustment.  It's commonly of the form "use
> $alternative instead".  The alternative is often a direct replacement,
> but not always.  There could even be no replacement at all.  We don't
> promise replacements, we promise an orderly process, so users can
> adjust.
>
> Sometimes, we don't have firm plans to remove, but are more like "maybe
> remove when gets in the way".  We could soften the "you need to adjust"
> signal in documentation, but I doubt that's a good idea.  Regardless,
> the need to help users adjust remains.
>
> Back to your patches.  There are two separate interfaces to block
> migration, and both are deprecated at the end of the series:
>
> 1. Migration parameter @block-incremental & friends
>
>Not in the way, content to keep around for longer if it helps users.
>
>The deprecation documentation advises to use block-mirror with NBD
>instead.  All good.
>
> 2. QMP migrate parameters @inc and @blk
>
>Firm intent to remove as soon as the grace period expires, because
>it's in the way.
>
>The deprecation documentation advises to use interface 1 instead.
>But that's deprecated, too!
>
>Insufficiently careful readers will miss that the replacement is
>deprecated, and just use it.  Risks surprise when the replacement
>goes away, too.
>
>More careful readers will realize that this advises to use something
>we elsewhere advise not to use.  Contradiction!  Confusion ensues.
>
>On further reflection, these readers might conclude that the
>*combined* advice is to use block-mirror with NBD instead.  This is
>correct.
>
>So why not tell them?
>
>Perhaps you'd like to give more nuanced advice, like "you should move
>to block-mirror with NBD, but if that's not practical for you, you
>should at least move to @block-incremental & friends, which will
>likely stick around for longer."  That's fine.  All I'm asking for is
>to not make things more confusing than they need to be :)
>
> [...]

Telling this in deprecated.rst is enough?  or you want me to put it also
in the error/warn messages and qapi?

Later, Juan.

Re: [PATCH 4/4] virSecretLoad: Simplify cleanup path

[Peter Krempa]

On Mon, Oct 16, 2023 at 10:07:51 +0200, Michal Privoznik wrote:
> When loading a secret value fails, the control jumps over to the
> 'cleanup' label where explicit call to virSecretDefFree()
> happens. This is unnecessary as the corresponding variable can be
> declared with g_autoptr() after which all error paths can just
> return NULL instantly.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/virsecretobj.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)

Reviewed-by: Peter Krempa 

Re: [PATCH 3/4] virSecretLoadAllConfigs: Use g_autofree for @path

[Peter Krempa]

On Mon, Oct 16, 2023 at 10:07:50 +0200, Michal Privoznik wrote:
> When loading virSecret configs, the @path variable holds path to
> individual config files. In each iteration it is freed explicitly
> using VIR_FREE(). Switch it to g_autofree and remove those
> explicit calls.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/virsecretobj.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Peter Krempa 

Re: [PATCH 2/4] virfile: Drop virBuildPathInternal()

[Peter Krempa]

On Mon, Oct 16, 2023 at 10:07:49 +0200, Michal Privoznik wrote:
> After previous cleanup the virBuildPathInternal() function is no
> longer used. Drop it.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/libvirt_private.syms |  1 -
>  src/util/virfile.c   | 22 --
>  src/util/virfile.h   |  5 -
>  3 files changed, 28 deletions(-)

Reviewed-by: Peter Krempa 

Re: [PATCH 1/4] lib: Replace virBuildPath() with g_build_filename()

[Peter Krempa]

On Mon, Oct 16, 2023 at 10:07:48 +0200, Michal Privoznik wrote:
> Our virBuildPath() constructs a path from given arguments.
> Exactly like g_build_filename(), except the latter is more
> generic as it uses backslashes on Windows. Therefore, replace the
> former with the latter.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  docs/kbase/internals/command.rst | 2 +-
>  src/util/virfcp.c| 2 +-
>  src/util/virhook.c   | 4 ++--
>  src/util/virpci.c| 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/kbase/internals/command.rst 
> b/docs/kbase/internals/command.rst
> index 738fb5930a..d958c9d16a 100644
> --- a/docs/kbase/internals/command.rst
> +++ b/docs/kbase/internals/command.rst
> @@ -444,7 +444,7 @@ src/util/hooks.c
>   g_autofree char *path = NULL;
>   g_autoptr(virCommand) cmd = NULL;
>  
> - virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr);
> + path = g_build_filename(LIBVIRT_HOOK_DIR, drvstr);

It's docs, but it's missing the NULL sentinel.

Reviewed-by: Peter Krempa 

Re: [PATCH v4 03/10] migration: migrate 'inc' command option is deprecated.

[Markus Armbruster]

Juan Quintela  writes:

> Markus Armbruster  wrote:
>> Juan Quintela  writes:
>>
>>> Set the 'block_incremental' migration parameter to 'true' instead.
>>>
>>> Reviewed-by: Thomas Huth 
>>> Acked-by: Stefan Hajnoczi 
>>> Signed-off-by: Juan Quintela 
>>>
>>> ---
>>>
>>> Improve documentation and style (thanks Markus)
>>> ---
>>>  docs/about/deprecated.rst | 7 +++
>>>  qapi/migration.json   | 8 +++-
>>>  migration/migration.c | 6 ++
>>>  3 files changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>>> index 1c4d7f36f0..1b6b2870cf 100644
>>> --- a/docs/about/deprecated.rst
>>> +++ b/docs/about/deprecated.rst
>>> @@ -452,3 +452,10 @@ Migration
>>>  ``skipped`` field in Migration stats has been deprecated.  It hasn't
>>>  been used for more than 10 years.
>>>  
>>> +``inc`` migrate command option (since 8.2)
>>> +''
>>> +
>>> +The new way to modify migration is using migration parameters.
>>> +``inc`` functionality can be achieved by setting the
>>> +``block-incremental`` migration parameter to ``true``.
>>> +
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 6865fea3c5..56bbd55b87 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -1492,6 +1492,11 @@
>>>  #
>>>  # @resume: resume one paused migration, default "off". (since 3.0)
>>>  #
>>> +# Features:
>>> +#
>>> +# @deprecated: Member @inc is deprecated.  Use migration parameter
>>> +# @block-incremental instead.
>>
>> This is fine now.  It becomes bad advice in PATCH 05, which deprecates
>> @block-incremental.  Two solutions:
>>
>> 1. Change this patch to point to an alternative that will *not* be
>> deprecated.
>
> Ok, clearly I am not explaining myself properly O:-)
>
> History of block migration:
> * In the beggining there was -b and -i migrate options
>   There was the only way to do storage of migration.
> * We moved to use parameters and capabilities for migration
>   So we created @block-incremental and @block.
>   But we didn't remove the command line options (for backward
>   compatibility).
> * We were asked to modify migration so some storaged was migrated and
>   some was not migrated during migration.  But block people found that
>   it was a good idea to allow storage migration without migrating the
>   vm, so they created this blockdev-mirror mechanism that is shinny,
>   funny, faster,  better.
>
> So now we have old code that basically nobody uses (the last big user
> was COLO, but now it can use multifd).  So we want to drop it, but we
> don't care about a direct replacement.
>
> So, why I am interested in removing this?
> - @block and @block-incremental: If you don't use block migration, their
>   existence don't bother you.  They are "quite" independent of the rest
>   of the migration code (they could be better integrated, but not big
>   trouble here).
> - migrate options -i/-b: This ones hurt us each time that we need to
>   do changing in options.  Notice that we have "perfect" replacements
>   with @block and @block-incremental, exactly the same
>   result/semantics/...
>   You can see the trobles in the RFC patches
>
>  * [PATCH v4 07/10] [RFC] migration: Make -i/-b an error for hmp and qmp
>  * [PATCH v4 08/10] [RFC] migration: Remove helpers needed for -i/-b 
> migrate options
>
> So what I want, I want to remove -i/-b in the next version (9.0?).  For
> the other, I want to remove it, but I don't care if the code is around
> in "deprecated" state for another couple of years if there are still
> people that feel that they want it.
>
> This is the reason that I put a pointer for -i/-b to
> @block/@block-incremental.  They are "perfect" replacements.
>
> I can put here to use blockdev-mirror + NBD, but the replacement is not
> so direct.
>
> Does this make sense?

I see where you're coming from.  Now let's change perspective for a
minute: what's the purpose of deprecating stuff?

We normally deprecate with intent to remove.

We don't remove right away, because we promised to first deprecate for a
grace period, so users can adjust in an orderly manner.  The deprecation
serves as signal "you need to adjust".  The documentation that comes
with it should help with the adjustment.  It's commonly of the form "use
$alternative instead".  The alternative is often a direct replacement,
but not always.  There could even be no replacement at all.  We don't
promise replacements, we promise an orderly process, so users can
adjust.

Sometimes, we don't have firm plans to remove, but are more like "maybe
remove when gets in the way".  We could soften the "you need to adjust"
signal in documentation, but I doubt that's a good idea.  Regardless,
the need to help users adjust remains.

Back to your patches.  There are two separate interfaces to block
migration, and both are deprecated at the end of the series:

1. Migration parameter @block-incremental & friends

   Not in the 

Re: [libvirt PATCH] hypervisor: Move interface mgmt methods to hypervisor

[Daniel P . Berrangé]

On Sun, Oct 15, 2023 at 03:03:40PM -0400, Laine Stump wrote:
> On 10/12/23 3:37 PM, Praveen K Paladugu wrote:
> > Move guest interface management methods from qemu to hypervisor. These
> > methods will be shared by networking support in ch driver.
> > 
> > Signed-off-by: Praveen K Paladugu 
> > ---
> >   po/POTFILES   |   1 +
> >   src/hypervisor/domain_interface.c | 279 ++
> >   src/hypervisor/domain_interface.h |  38 
> >   src/hypervisor/meson.build|   1 +
> >   src/libvirt_private.syms  |   6 +
> >   src/qemu/qemu_command.c   |   7 +-
> >   src/qemu/qemu_hotplug.c   |   3 +-
> >   src/qemu/qemu_interface.c | 246 +-
> >   src/qemu/qemu_interface.h |   6 -
> >   src/qemu/qemu_process.c   |   3 +-
> >   10 files changed, 341 insertions(+), 249 deletions(-)
> >   create mode 100644 src/hypervisor/domain_interface.c
> >   create mode 100644 src/hypervisor/domain_interface.h
> > 
> > diff --git a/po/POTFILES b/po/POTFILES
> > index 3a51aea5cb..023c041f61 100644
> > --- a/po/POTFILES
> > +++ b/po/POTFILES
> > @@ -92,6 +92,7 @@ src/hyperv/hyperv_util.c
> >   src/hyperv/hyperv_wmi.c
> >   src/hypervisor/domain_cgroup.c
> >   src/hypervisor/domain_driver.c
> > +src/hypervisor/domain_interface.c
> >   src/hypervisor/virhostdev.c
> >   src/interface/interface_backend_netcf.c
> >   src/interface/interface_backend_udev.c
> > diff --git a/src/hypervisor/domain_interface.c 
> > b/src/hypervisor/domain_interface.c
> > new file mode 100644
> > index 00..b01b6ee767
> > --- /dev/null
> > +++ b/src/hypervisor/domain_interface.c
> > @@ -0,0 +1,279 @@
> > +/*
> > + * Copyright Microsoft Corp. 2023
> 
> I haven't had time to look through the rest of this yet (although it seems
> pretty straightforward, and I think it might have been me who suggested it
> in the first place :-)), I did want to bring up this one topic now:
> 
> 
> Best practices for the copyright notice in a new file have changed over the
> years (for example, we no longer list the "Author" of new files and even
> removed Author from existing files, since a more accurate and complete
> version of that info is all available from git), and I haven't paid close
> attention, but since this entire file is comprised of functions that were
> just moved from an existing file and renamed (rather than actual new code),
> I don't think it's appropriate for it to erase all trace of the original
> copyright holder and simply assign the copyright entirely to Microsoft.

If the original file that the code was copied from had any
Copyright lines, those must be preserved in the new file.
If the original file had no copyright lines, you're not
required to do any archeology to figure out copyrighth
holders, just leave the new file similarly devoid.

Simply moving code is also not a copyrightable enhancement,
so adding your own copyright on moved code is inappropriate.

Once you add new functionality to the newfile though, you
are free to add your own copyright statement at that point,
if desired.


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

[PATCH 3/4] virSecretLoadAllConfigs: Use g_autofree for @path

[Michal Privoznik]

When loading virSecret configs, the @path variable holds path to
individual config files. In each iteration it is freed explicitly
using VIR_FREE(). Switch it to g_autofree and remove those
explicit calls.

Signed-off-by: Michal Privoznik 
---
 src/conf/virsecretobj.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index e0393e6cec..2585b2c972 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -902,7 +902,7 @@ virSecretLoadAllConfigs(virSecretObjList *secrets,
 /* Ignore errors reported by readdir or other calls within the
  * loop (if any).  It's better to keep the secrets we managed to find. */
 while (virDirRead(dir, &de, NULL) > 0) {
-char *path;
+g_autofree char *path = NULL;
 virSecretObj *obj;
 
 if (!virStringHasSuffix(de->d_name, ".xml"))
@@ -914,11 +914,9 @@ virSecretLoadAllConfigs(virSecretObjList *secrets,
 if (!(obj = virSecretLoad(secrets, de->d_name, path, configDir))) {
 VIR_ERROR(_("Error reading secret: %1$s"),
   virGetLastErrorMessage());
-VIR_FREE(path);
 continue;
 }
 
-VIR_FREE(path);
 virSecretObjEndAPI(&obj);
 }
 
-- 
2.41.0

[PATCH 4/4] virSecretLoad: Simplify cleanup path

[Michal Privoznik]

When loading a secret value fails, the control jumps over to the
'cleanup' label where explicit call to virSecretDefFree()
happens. This is unnecessary as the corresponding variable can be
declared with g_autoptr() after which all error paths can just
return NULL instantly.

Signed-off-by: Michal Privoznik 
---
 src/conf/virsecretobj.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 2585b2c972..455798d414 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -865,25 +865,24 @@ virSecretLoad(virSecretObjList *secrets,
   const char *path,
   const char *configDir)
 {
-virSecretDef *def = NULL;
+g_autoptr(virSecretDef) def = NULL;
 virSecretObj *obj = NULL;
 
 if (!(def = virSecretDefParse(NULL, path, 0)))
-goto cleanup;
+return NULL;
 
 if (virSecretLoadValidateUUID(def, file) < 0)
-goto cleanup;
+return NULL;
 
 if (!(obj = virSecretObjListAdd(secrets, &def, configDir, NULL)))
-goto cleanup;
+return NULL;
 
 if (virSecretLoadValue(obj) < 0) {
 virSecretObjListRemove(secrets, obj);
 g_clear_pointer(&obj, virObjectUnref);
+return NULL;
 }
 
- cleanup:
-virSecretDefFree(def);
 return obj;
 }
 
-- 
2.41.0

[PATCH 2/4] virfile: Drop virBuildPathInternal()

[Michal Privoznik]

After previous cleanup the virBuildPathInternal() function is no
longer used. Drop it.

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |  1 -
 src/util/virfile.c   | 22 --
 src/util/virfile.h   |  5 -
 3 files changed, 28 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4e475d5b1a..553b01b8c0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2280,7 +2280,6 @@ virFDStreamSetInternalCloseCb;
 saferead;
 safewrite;
 safezero;
-virBuildPathInternal;
 virCloseFrom;
 virCloseRange;
 virCloseRangeInit;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index bd36a9a31a..54708652fb 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1490,28 +1490,6 @@ virFileFindMountPoint(const char *type G_GNUC_UNUSED)
 
 #endif /* defined WITH_MNTENT_H && defined WITH_GETMNTENT_R */
 
-void
-virBuildPathInternal(char **path, ...)
-{
-char *path_component = NULL;
-g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-va_list ap;
-
-va_start(ap, path);
-
-path_component = va_arg(ap, char *);
-virBufferAdd(&buf, path_component, -1);
-
-while ((path_component = va_arg(ap, char *)) != NULL) {
-virBufferAddChar(&buf, '/');
-virBufferAdd(&buf, path_component, -1);
-}
-
-va_end(ap);
-
-*path = virBufferContentAndReset(&buf);
-}
-
 /* Read no more than the specified maximum number of bytes. */
 static char *
 saferead_lim(int fd, size_t max_len, size_t *length)
diff --git a/src/util/virfile.h b/src/util/virfile.h
index adc032ba33..286401e0f5 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -307,11 +307,6 @@ int virFileOpenTty(int *ttymaster,
 
 char *virFileFindMountPoint(const char *type);
 
-/* NB: this should be combined with virFileBuildPath */
-#define virBuildPath(path, ...) \
-virBuildPathInternal(path, __VA_ARGS__, NULL)
-void virBuildPathInternal(char **path, ...) G_GNUC_NULL_TERMINATED;
-
 typedef struct _virHugeTLBFS virHugeTLBFS;
 struct _virHugeTLBFS {
 char *mnt_dir;  /* Where the FS is mount to */
-- 
2.41.0

[PATCH 1/4] lib: Replace virBuildPath() with g_build_filename()

[Michal Privoznik]

Our virBuildPath() constructs a path from given arguments.
Exactly like g_build_filename(), except the latter is more
generic as it uses backslashes on Windows. Therefore, replace the
former with the latter.

Signed-off-by: Michal Privoznik 
---
 docs/kbase/internals/command.rst | 2 +-
 src/util/virfcp.c| 2 +-
 src/util/virhook.c   | 4 ++--
 src/util/virpci.c| 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/docs/kbase/internals/command.rst b/docs/kbase/internals/command.rst
index 738fb5930a..d958c9d16a 100644
--- a/docs/kbase/internals/command.rst
+++ b/docs/kbase/internals/command.rst
@@ -444,7 +444,7 @@ src/util/hooks.c
  g_autofree char *path = NULL;
  g_autoptr(virCommand) cmd = NULL;
 
- virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr);
+ path = g_build_filename(LIBVIRT_HOOK_DIR, drvstr);
 
  cmd = virCommandNew(path);
 
diff --git a/src/util/virfcp.c b/src/util/virfcp.c
index bb62fa9025..052a6c99e1 100644
--- a/src/util/virfcp.c
+++ b/src/util/virfcp.c
@@ -38,7 +38,7 @@ virFCIsCapableRport(const char *rport)
 {
 g_autofree char *path = NULL;
 
-virBuildPath(&path, SYSFS_FC_RPORT_PATH, rport);
+path = g_build_filename(SYSFS_FC_RPORT_PATH, rport, NULL);
 
 return virFileExists(path);
 }
diff --git a/src/util/virhook.c b/src/util/virhook.c
index 50e178723f..d012bb1825 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -153,7 +153,7 @@ virHookCheck(int no, const char *driver)
 return -1;
 }
 
-virBuildPath(&path, LIBVIRT_HOOK_DIR, driver);
+path = g_build_filename(LIBVIRT_HOOK_DIR, driver, NULL);
 
 if (!virFileExists(path)) {
 VIR_DEBUG("No hook script %s", path);
@@ -398,7 +398,7 @@ virHookCall(int driver,
 if (extra == NULL)
 extra = "-";
 
-virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr);
+path = g_build_filename(LIBVIRT_HOOK_DIR, drvstr, NULL);
 
 script_ret = 1;
 
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 08b82708b1..baacde4c14 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2396,7 +2396,7 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path,
 
 *pf = NULL;
 
-virBuildPath(&device_link, vf_sysfs_path, "physfn");
+device_link = g_build_filename(vf_sysfs_path, "physfn", NULL);
 
 if ((*pf = virPCIGetDeviceAddressFromSysfsLink(device_link))) {
 VIR_DEBUG("PF for VF device '%s': " VIR_PCI_DEVICE_ADDRESS_FMT,
@@ -2580,7 +2580,7 @@ virPCIGetNetName(const char *device_link_sysfs_path,
 return -1;
 }
 
-virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path, "net");
+pcidev_sysfs_net_path = g_build_filename(device_link_sysfs_path, "net", 
NULL);
 
 if (virDirOpenQuiet(&dir, pcidev_sysfs_net_path) < 0) {
 /* this *isn't* an error - caller needs to check for netname == NULL */
-- 
2.41.0

[PATCH 0/4] Misc cleanups

[Michal Privoznik]

*** BLURB HERE ***

Michal Prívozník (4):
  lib: Replace virBuildPath() with g_build_filename()
  virfile: Drop virBuildPathInternal()
  virSecretLoadAllConfigs: Use g_autofree for @path
  virSecretLoad: Simplify cleanup path

 docs/kbase/internals/command.rst |  2 +-
 src/conf/virsecretobj.c  | 15 ++-
 src/libvirt_private.syms |  1 -
 src/util/virfcp.c|  2 +-
 src/util/virfile.c   | 22 --
 src/util/virfile.h   |  5 -
 src/util/virhook.c   |  4 ++--
 src/util/virpci.c|  4 ++--
 8 files changed, 12 insertions(+), 43 deletions(-)

-- 
2.41.0

Re: [PATCH] virDomainMemoryDefValidate: Skip the same device on validation on memory device update

[Ján Tomko]

On a Monday in 2023, Michal Privoznik wrote:

In my recent commit of v9.8.0-rc1~7 I've introduced validation
wrt other memory devices. And mostly works, except when doing
memory device update ('virsh update-memory-device') because then
@mem is just parsed  device XML and thus its pointer is
not in the vm->def->mem, yet. Thus my algorithm which skips over
the same entry fails. Fortunately, we require full device XML on
device update and thus we can use device address and aliases to
detect duplicity.

Fixes: 3fd64fb0e236fc80ffa2cc977c0d471f11fc39bf
Signed-off-by: Michal Privoznik 
---
src/conf/domain_validate.c | 15 +++
1 file changed, 15 insertions(+)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

[PATCH] virDomainMemoryDefValidate: Skip the same device on validation on memory device update

[Michal Privoznik]

In my recent commit of v9.8.0-rc1~7 I've introduced validation
wrt other memory devices. And mostly works, except when doing
memory device update ('virsh update-memory-device') because then
@mem is just parsed  device XML and thus its pointer is
not in the vm->def->mem, yet. Thus my algorithm which skips over
the same entry fails. Fortunately, we require full device XML on
device update and thus we can use device address and aliases to
detect duplicity.

Fixes: 3fd64fb0e236fc80ffa2cc977c0d471f11fc39bf
Signed-off-by: Michal Privoznik 
---
 src/conf/domain_validate.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 6962fe76bf..bc3d00f89c 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2387,6 +2387,21 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
 if (other == mem)
 continue;
 
+/* In case we're updating an existing memory device (e.g. virtio-mem),
+ * then pointers will be different. But addresses and aliases are the
+ * same. However, STREQ_NULLABLE() returns true if both strings are
+ * NULL which is not what we want. */
+if (virDomainDeviceInfoAddressIsEqual(&other->info,
+  &mem->info)) {
+continue;
+}
+
+if (mem->info.alias &&
+STREQ_NULLABLE(other->info.alias,
+   mem->info.alias)) {
+continue;
+}
+
 switch (other->model) {
 case VIR_DOMAIN_MEMORY_MODEL_NONE:
 case VIR_DOMAIN_MEMORY_MODEL_DIMM:
-- 
2.41.0

Re: [PATCH v4 03/10] migration: migrate 'inc' command option is deprecated.

[Juan Quintela]

Markus Armbruster  wrote:
> Juan Quintela  writes:
>
>> Set the 'block_incremental' migration parameter to 'true' instead.
>>
>> Reviewed-by: Thomas Huth 
>> Acked-by: Stefan Hajnoczi 
>> Signed-off-by: Juan Quintela 
>>
>> ---
>>
>> Improve documentation and style (thanks Markus)
>> ---
>>  docs/about/deprecated.rst | 7 +++
>>  qapi/migration.json   | 8 +++-
>>  migration/migration.c | 6 ++
>>  3 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 1c4d7f36f0..1b6b2870cf 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -452,3 +452,10 @@ Migration
>>  ``skipped`` field in Migration stats has been deprecated.  It hasn't
>>  been used for more than 10 years.
>>  
>> +``inc`` migrate command option (since 8.2)
>> +''
>> +
>> +The new way to modify migration is using migration parameters.
>> +``inc`` functionality can be achieved by setting the
>> +``block-incremental`` migration parameter to ``true``.
>> +
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 6865fea3c5..56bbd55b87 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1492,6 +1492,11 @@
>>  #
>>  # @resume: resume one paused migration, default "off". (since 3.0)
>>  #
>> +# Features:
>> +#
>> +# @deprecated: Member @inc is deprecated.  Use migration parameter
>> +# @block-incremental instead.
>
> This is fine now.  It becomes bad advice in PATCH 05, which deprecates
> @block-incremental.  Two solutions:
>
> 1. Change this patch to point to an alternative that will *not* be
> deprecated.

Ok, clearly I am not explaining myself properly O:-)

History of block migration:
* In the beggining there was -b and -i migrate options
  There was the only way to do storage of migration.
* We moved to use parameters and capabilities for migration
  So we created @block-incremental and @block.
  But we didn't remove the command line options (for backward
  compatibility).
* We were asked to modify migration so some storaged was migrated and
  some was not migrated during migration.  But block people found that
  it was a good idea to allow storage migration without migrating the
  vm, so they created this blockdev-mirror mechanism that is shinny,
  funny, faster,  better.

So now we have old code that basically nobody uses (the last big user
was COLO, but now it can use multifd).  So we want to drop it, but we
don't care about a direct replacement.

So, why I am interested in removing this?
- @block and @block-incremental: If you don't use block migration, their
  existence don't bother you.  They are "quite" independent of the rest
  of the migration code (they could be better integrated, but not big
  trouble here).
- migrate options -i/-b: This ones hurt us each time that we need to
  do changing in options.  Notice that we have "perfect" replacements
  with @block and @block-incremental, exactly the same
  result/semantics/...
  You can see the trobles in the RFC patches

 * [PATCH v4 07/10] [RFC] migration: Make -i/-b an error for hmp and qmp
 * [PATCH v4 08/10] [RFC] migration: Remove helpers needed for -i/-b 
migrate options

So what I want, I want to remove -i/-b in the next version (9.0?).  For
the other, I want to remove it, but I don't care if the code is around
in "deprecated" state for another couple of years if there are still
people that feel that they want it.

This is the reason that I put a pointer for -i/-b to
@block/@block-incremental.  They are "perfect" replacements.

I can put here to use blockdev-mirror + NBD, but the replacement is not
so direct.

Does this make sense?


> 2. Change PATCH 05.
>
> Same end result.
>
>> +#
>>  # Returns: nothing on success
>>  #
>>  # Since: 0.14
>> @@ -1513,7 +1518,8 @@
>>  # <- { "return": {} }
>>  ##
>>  { 'command': 'migrate',
>> -  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool',
>> +  'data': {'uri': 'str', '*blk': 'bool',
>> +   '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] },
>> '*detach': 'bool', '*resume': 'bool' } }
>>  
>>  ##
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 1c6c81ad49..ac4897fe0d 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1601,6 +1601,12 @@ static bool migrate_prepare(MigrationState *s, bool 
>> blk, bool blk_inc,
>>  {
>>  Error *local_err = NULL;
>>  
>> +if (blk_inc) {
>> +warn_report("@inc/-i migrate option is deprecated, set the"
>
> This is either about QMP migrate's parameter "inc", or HMP migrate's
> flags -i.

Needs to be @inc.  I want about the "-i" command option in other place.

> In the former case, we want something like "parameter 'inc' is
> deprecated".

This one.

> In the latter case, we want something like "-i is deprecated".

Ok, changing.

> Trying to do both in a single message results in a sub-par message.  If
> you want to do better, y