Re: [PULL 20/39] hw/s390x/s390-stattrib: Simplify handling of the "migration-enabled" property

2023-10-26 Thread Thomas Huth

On 24/10/2023 15.12, Juan Quintela wrote:

From: Thomas Huth 

There's no need for dedicated handlers here if they don't do anything
special.

Acked-by: David Hildenbrand 
Reviewed-by: Eric Farman 
Acked-by: Juan Quintela 
Signed-off-by: Thomas Huth 
Signed-off-by: Juan Quintela 
Message-ID: <20231020150554.664422-3-th...@redhat.com>
---
  hw/s390x/s390-stattrib.c | 26 ++
  1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index 220e845d12..52f9fc036e 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -13,6 +13,7 @@
  #include "qemu/units.h"
  #include "migration/qemu-file.h"
  #include "migration/register.h"
+#include "hw/qdev-properties.h"
  #include "hw/s390x/storage-attributes.h"
  #include "qemu/error-report.h"
  #include "exec/ram_addr.h"
@@ -340,6 +341,10 @@ static void s390_stattrib_realize(DeviceState *dev, Error 
**errp)
  }
  }
  
+static Property s390_stattrib_props[] = {

+DEFINE_PROP_BOOL("migration-enabled", S390StAttribState, 
migration_enabled, true),


This needs a  DEFINE_PROP_END_OF_LIST() here, too ... sorry for that!
/me is looking for his brown paper-bags...

 Thomas


+};
+
  static void s390_stattrib_class_init(ObjectClass *oc, void *data)
  {
  DeviceClass *dc = DEVICE_CLASS(oc);
@@ -347,22 +352,7 @@ static void s390_stattrib_class_init(ObjectClass *oc, void 
*data)
  dc->hotpluggable = false;
  set_bit(DEVICE_CATEGORY_MISC, dc->categories);
  dc->realize = s390_stattrib_realize;
-}
-
-static inline bool s390_stattrib_get_migration_enabled(Object *obj,
-   Error **errp)
-{
-S390StAttribState *s = S390_STATTRIB(obj);
-
-return s->migration_enabled;
-}
-
-static inline void s390_stattrib_set_migration_enabled(Object *obj, bool value,
-Error **errp)
-{
-S390StAttribState *s = S390_STATTRIB(obj);
-
-s->migration_enabled = value;
+device_class_set_props(dc, s390_stattrib_props);
  }
  
  static SaveVMHandlers savevm_s390_stattrib_handlers = {

@@ -383,10 +373,6 @@ static void s390_stattrib_instance_init(Object *obj)
  register_savevm_live(TYPE_S390_STATTRIB, 0, 0,
   &savevm_s390_stattrib_handlers, sas);
  
-object_property_add_bool(obj, "migration-enabled",

- s390_stattrib_get_migration_enabled,
- s390_stattrib_set_migration_enabled);
-object_property_set_bool(obj, "migration-enabled", true, NULL);
  sas->migration_cur_gfn = 0;
  }
  




Re: [PULL 19/39] hw/s390x/s390-skeys: Don't call register_savevm_live() during instance_init()

2023-10-26 Thread Thomas Huth

On 24/10/2023 15.12, Juan Quintela wrote:

From: Thomas Huth 

Since the instance_init() function immediately tries to set the
property to "true", the s390_skeys_set_migration_enabled() tries
to register a savevm handler during instance_init(). However,
instance_init() functions can be called multiple times, e.g. for
introspection of devices. That means multiple instances of devices
can be created during runtime (which is fine as long as they all
don't get realized, too), so the "Prevent double registration of
savevm handler" check in the s390_skeys_set_migration_enabled()
function does not work at all as expected (since there could be
more than one instance).

Thus we must not call register_savevm_live() from an instance_init()
function at all. Move this to the realize() function instead. This
way we can also get rid of the property getter and setter functions
completely, simplifying the code along the way quite a bit.

Acked-by: David Hildenbrand 
Reviewed-by: Eric Farman 
Acked-by: Juan Quintela 
Signed-off-by: Thomas Huth 
Signed-off-by: Juan Quintela 
Message-ID: <20231020150554.664422-2-th...@redhat.com>
---
  hw/s390x/s390-skeys.c | 35 ---
  1 file changed, 8 insertions(+), 27 deletions(-)

diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 5024faf411..8e9d9e41e8 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -12,6 +12,7 @@
  #include "qemu/osdep.h"
  #include "qemu/units.h"
  #include "hw/boards.h"
+#include "hw/qdev-properties.h"
  #include "hw/s390x/storage-keys.h"
  #include "qapi/error.h"
  #include "qapi/qapi-commands-misc-target.h"
@@ -432,58 +433,38 @@ static int s390_storage_keys_load(QEMUFile *f, void 
*opaque, int version_id)
  return ret;
  }
  
-static inline bool s390_skeys_get_migration_enabled(Object *obj, Error **errp)

-{
-S390SKeysState *ss = S390_SKEYS(obj);
-
-return ss->migration_enabled;
-}
-
  static SaveVMHandlers savevm_s390_storage_keys = {
  .save_state = s390_storage_keys_save,
  .load_state = s390_storage_keys_load,
  };
  
-static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,

-Error **errp)
+static void s390_skeys_realize(DeviceState *dev, Error **errp)
  {
-S390SKeysState *ss = S390_SKEYS(obj);
-
-/* Prevent double registration of savevm handler */
-if (ss->migration_enabled == value) {
-return;
-}
-
-ss->migration_enabled = value;
+S390SKeysState *ss = S390_SKEYS(dev);
  
  if (ss->migration_enabled) {

  register_savevm_live(TYPE_S390_SKEYS, 0, 1,
   &savevm_s390_storage_keys, ss);
-} else {
-unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
  }
  }
  
-static void s390_skeys_instance_init(Object *obj)

-{
-object_property_add_bool(obj, "migration-enabled",
- s390_skeys_get_migration_enabled,
- s390_skeys_set_migration_enabled);
-object_property_set_bool(obj, "migration-enabled", true, NULL);
-}
+static Property s390_skeys_props[] = {
+DEFINE_PROP_BOOL("migration-enabled", S390SKeysState, migration_enabled, 
true),


This needs a  DEFINE_PROP_END_OF_LIST() here ... mea culpa!

 Thomas



+};
  
  static void s390_skeys_class_init(ObjectClass *oc, void *data)

  {
  DeviceClass *dc = DEVICE_CLASS(oc);
  
  dc->hotpluggable = false;

+dc->realize = s390_skeys_realize;
+device_class_set_props(dc, s390_skeys_props);
  set_bit(DEVICE_CATEGORY_MISC, dc->categories);
  }
  
  static const TypeInfo s390_skeys_info = {

  .name  = TYPE_S390_SKEYS,
  .parent= TYPE_DEVICE,
-.instance_init = s390_skeys_instance_init,
  .instance_size = sizeof(S390SKeysState),
  .class_init= s390_skeys_class_init,
  .class_size= sizeof(S390SKeysClass),




Re: [PULL 00/39] Migration 20231024 patches

2023-10-26 Thread Juan Quintela
Stefan Hajnoczi  wrote:
> On Tue, 24 Oct 2023 at 23:45, Juan Quintela  wrote:
>>
>> The following changes since commit a95260486aa7e78d7c7194eba65cf03311ad94ad:
>>
>>   Merge tag 'pull-tcg-20231023' of https://gitlab.com/rth7680/qemu into 
>> staging (2023-10-23 14:45:46 -0700)
>>
>> are available in the Git repository at:
>>
>>   https://gitlab.com/juan.quintela/qemu.git 
>> tags/migration-20231024-pull-request
>>
>> for you to fetch changes up to 088f7f03da3f5b3487091302b795c22b1bfe56fb:
>>
>>   migration: Deprecate old compression method (2023-10-24 13:48:24 +0200)
>>
>> 
>> Migration Pull request (20231024)
>>
>> Hi
>>
>> In this PULL:
>> - vmstate registration fixes (thomas, juan)
>> - start merging vmstate_section_needed changes (marc)
>> - migration depreactions (juan)
>> - migration documentation for backwards compatibility (juan)
>>
>> Please apply.
>
> Hi Juan,
> I'm seeing CI failures:
> https://gitlab.com/qemu-project/qemu/-/pipelines/1048630760

start with s390x:

Errors:

 32/840 qemu:qtest+qtest-s390x / qtest-s390x/qom-test   
  ERROR  50.27s   killed by signal 6 SIGABRT
104/840 qemu:qtest+qtest-s390x / qtest-s390x/test-hmp   
  ERROR  51.55s   killed by signal 6 SIGABRT
189/840 qemu:qtest+qtest-s390x / qtest-s390x/boot-serial-test   
  ERROR  54.07s   killed by signal 6 SIGABRT
192/840 qemu:qtest+qtest-s390x / qtest-s390x/qos-test   
  ERROR  51.29s   killed by signal 6 SIGABRT
519/840 qemu:qtest+qtest-s390x / qtest-s390x/test-filter-mirror 
  ERROR  50.36s   killed by signal 6 SIGABRT
520/840 qemu:qtest+qtest-s390x / qtest-s390x/test-netfilter 
  ERROR  51.03s   killed by signal 6 SIGABRT
522/840 qemu:qtest+qtest-s390x / qtest-s390x/device-plug-test   
  ERROR  50.99s   killed by signal 6 SIGABRT
523/840 qemu:qtest+qtest-s390x / qtest-s390x/test-filter-redirector 
  ERROR  54.14s   killed by signal 6 SIGABRT
524/840 qemu:qtest+qtest-s390x / qtest-s390x/drive_del-test 
  ERROR  53.40s   killed by signal 6 SIGABRT
525/840 qemu:qtest+qtest-s390x / qtest-s390x/virtio-ccw-test
  ERROR  54.67s   killed by signal 6 SIGABRT
526/840 qemu:qtest+qtest-s390x / qtest-s390x/device-introspect-test 
  ERROR  51.15s   killed by signal 6 SIGABRT
527/840 qemu:qtest+qtest-s390x / qtest-s390x/cpu-plug-test  
  ERROR  51.21s   killed by signal 6 SIGABRT
535/840 qemu:qtest+qtest-s390x / qtest-s390x/qmp-test   
  ERROR  51.18s   killed by signal 6 SIGABRT
534/840 qemu:qtest+qtest-s390x / qtest-s390x/machine-none-test  
  ERROR  51.21s   killed by signal 6 SIGABRT
533/840 qemu:qtest+qtest-s390x / qtest-s390x/qmp-cmd-test   
  ERROR  51.22s   killed by signal 6 SIGABRT
549/840 qemu:qtest+qtest-s390x / qtest-s390x/readconfig-test
  ERROR  51.20s   killed by signal 6 SIGABRT
644/840 qemu:block / io-qcow2-001   
  ERROR   0.32s   exit status 1
645/840 qemu:block / io-qcow2-002   
  ERROR   0.32s   exit status 1
646/840 qemu:block / io-qcow2-003   
  ERROR   0.34s   exit status 1
647/840 qemu:block / io-qcow2-004   
  ERROR   0.31s   exit status 1
648/840 qemu:block / io-qcow2-005   
  ERROR   0.43s   exit status 1
649/840 qemu:block / io-qcow2-007   
  ERROR   0.34s   exit status 1
650/840 qemu:block / io-qcow2-008   
  ERROR   0.63s   exit status 1
651/840 qemu:block / io-qcow2-009   
  ERROR   0.32s   exit status 1
652/840 qemu:block / io-qcow2-010   
  ERROR   0.30s   exit status 1
654/840 qemu:block / io-qcow2-011   
  ERROR   0.31s   exit status 1
655/840 qemu:block / io-qcow2-012   
  ERROR   0.36s   exit status 1
657/840 qemu:block / io-qcow2-013   
  ERROR   0.51s   exit status 1
658/840 qemu:block / io-qcow2-017   
  ERROR   0.37s   exit status 1
659/840 qemu:block / io-qcow2-018   
  ERROR   0.31s   exit status 1
660/840 qemu:block / io-qcow2-019   
  ERROR

Re: RFC: Switch to a date-based versioning scheme

2023-10-26 Thread Daniel P . Berrangé
On Thu, Oct 26, 2023 at 06:48:28AM -0700, Andrea Bolognani wrote:
> Since we're just a few months away from the 10.0.0 release, I thought
> it would be a good time to bring up this idea.
> 
> Can we move to date-based version numbers? I suggest having
> 
>   libvirt 24.01.0 instead of 10.0.0
>   24.03.010.1.0
>   24.04.010.2.0
>  ...
>   24.11.010.9.0
>   24.12.010.10.0
> 
> The big advantage is that, once version numbers are obviously
> date-based, any expectation of them being interpreted according to
> semver[1] are immediately gone.

I don't think that is the case. Any version number you ever
see can plausibly be a semver version when seen without any
context. The only way that confusion goes away is if we were
to actually use semver, or if people stop blindly assuming
every project uses semver. I don't think this is a reason
to change what we're doing.

> People are quite used to date-based version numbers thanks to Ubuntu
> having used them for almost two decades, so I don't think anyone is
> going to be confused by the move. And since our release schedule is
> already date-based, having the versioning scheme match that just
> makes perfect sense IMO.
> 
> Up until now, one could have argued in favor of the current
> versioning scheme because of the single-digit major version
> component, but that's going away next year regardless, which makes
> this the perfect time to raise the topic :)
> 
> Thoughts?

My thoughts on calver are unchanged since it was previously
suggested this

  https://listman.redhat.com/archives/libvir-list/2016-June/131961.html
  https://listman.redhat.com/archives/libvir-list/2016-June/131958.html

Our version numbers are explicitly just a counter that ticks
over on a defined schedule and semantic meaning should not be
attached to any single release number.

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 :|



RFC: Switch to a date-based versioning scheme

2023-10-26 Thread Andrea Bolognani
Since we're just a few months away from the 10.0.0 release, I thought
it would be a good time to bring up this idea.

Can we move to date-based version numbers? I suggest having

  libvirt 24.01.0 instead of 10.0.0
  24.03.010.1.0
  24.04.010.2.0
 ...
  24.11.010.9.0
  24.12.010.10.0

The big advantage is that, once version numbers are obviously
date-based, any expectation of them being interpreted according to
semver[1] are immediately gone.

Of course semver doesn't make sense for us, given our extremely
strong backwards compatibility guarantees, and that's exactly why
we've left it behind with 2.0.0; however, that's something that's not
immediately obvious to someone who's not very involved with our
development process, and regarless of our intentions libvirt version
numbers *will* be mistakenly assumed to be semver-compliant on
occasion.

People are quite used to date-based version numbers thanks to Ubuntu
having used them for almost two decades, so I don't think anyone is
going to be confused by the move. And since our release schedule is
already date-based, having the versioning scheme match that just
makes perfect sense IMO.

Up until now, one could have argued in favor of the current
versioning scheme because of the single-digit major version
component, but that's going away next year regardless, which makes
this the perfect time to raise the topic :)

Thoughts?


[1] https://semver.org/
-- 
Andrea Bolognani / Red Hat / Virtualization



[PATCH v2 3/3] qemu: Setup host side of VDPA device for block copy

2023-10-26 Thread Peter Krempa
Setup the VDPA bits of the appropriate part of the image chain for block
copy.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c  |  4 
 src/qemu/qemu_driver.c | 12 
 2 files changed, 16 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 41038fb994..42c12a5e9b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -23,6 +23,7 @@
 #include "qemu_domain.h"
 #include "qemu_alias.h"
 #include "qemu_security.h"
+#include "qemu_process.h"

 #include "storage_source.h"
 #include "viralloc.h"
@@ -3675,6 +3676,9 @@ qemuBlockPivot(virDomainObj *vm,
 virQEMUCapsGet(priv->qemuCaps, 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY) &&
 virStorageSourceHasBacking(disk->mirror)) {

+if (qemuProcessPrepareHostStorageSourceChain(vm, 
disk->mirror->backingStore) < 0)
+return -1;
+
 if (!(chainattachdata = 
qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->mirror->backingStore)))
 return -1;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 86da8da777..d00d2a27c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14290,10 +14290,16 @@ qemuDomainBlockCopyCommon(virDomainObj *vm,
 if (virQEMUCapsGet(priv->qemuCaps, 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY)) {
 g_autoptr(virStorageSource) terminator = virStorageSourceNew();

+if (qemuProcessPrepareHostStorageSource(vm, mirror) < 0)
+goto endjob;
+
 if (!(data = 
qemuBuildStorageSourceChainAttachPrepareBlockdevTop(mirror,
  
terminator)))
 goto endjob;
 } else {
+if (qemuProcessPrepareHostStorageSourceChain(vm, mirror) < 0)
+goto endjob;
+
 if (!(data = 
qemuBuildStorageSourceChainAttachPrepareBlockdev(mirror)))
 goto endjob;
 }
@@ -14308,6 +14314,9 @@ qemuDomainBlockCopyCommon(virDomainObj *vm,
 if (mirror_shallow) {
 /* if external backing store is populated we'll need to open it */
 if (virStorageSourceHasBacking(mirror)) {
+if (qemuProcessPrepareHostStorageSourceChain(vm, 
mirror->backingStore) < 0)
+goto endjob;
+
 if (!(data = 
qemuBuildStorageSourceChainAttachPrepareBlockdev(mirror->backingStore)))
 goto endjob;

@@ -14321,6 +14330,9 @@ qemuDomainBlockCopyCommon(virDomainObj *vm,
 mirrorBacking = mirror->backingStore;
 }

+if (qemuProcessPrepareHostStorageSource(vm, mirror) < 0)
+goto endjob;
+
 if (!(crdata = 
qemuBuildStorageSourceChainAttachPrepareBlockdevTop(mirror,

mirrorBacking)))
 goto endjob;
-- 
2.41.0



[PATCH v2 1/3] qemu: process: Extract host setup of disk device into helpers

2023-10-26 Thread Peter Krempa
Currently the code sets up only VDPA backends but will be used later in
hotplug code too.

This patch also uses normal forward iteration in the loop in
qemuProcessPrepareHostStorage as we don't need to remove disks from the
disk list at that point.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_process.c | 76 -
 src/qemu/qemu_process.h |  7 
 2 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 63c0c62a46..1ef032dbd2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6777,6 +6777,69 @@ qemuProcessPrepareHostStorageSourceVDPA(virStorageSource 
*src,
 }


+/**
+ * See qemuProcessPrepareHostStorageSourceChain
+ */
+int
+qemuProcessPrepareHostStorageSource(virDomainObj *vm,
+virStorageSource *src)
+{
+/* connect to any necessary vdpa block devices */
+if (qemuProcessPrepareHostStorageSourceVDPA(src, vm->privateData) < 0)
+return -1;
+
+return 0;
+}
+
+
+/**
+ * qemuProcessPrepareHostStorageSourceChain:
+ *
+ * @vm: domain object
+ * @chain: source chain
+ *
+ * Prepare the host side of a disk for use with the VM. Note that this function
+ * accesses host resources.
+ */
+int
+qemuProcessPrepareHostStorageSourceChain(virDomainObj *vm,
+ virStorageSource *chain)
+{
+virStorageSource *n;
+
+for (n = chain; virStorageSourceIsBacking(n); n = n->backingStore) {
+if (qemuProcessPrepareHostStorageSource(vm, n) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
+/**
+ * qemuProcessPrepareHostStorageDisk:
+ *
+ * @vm: domain object
+ * @disk: disk definition object
+ *
+ * Prepare the host side of a disk for use with the VM. Note that this function
+ * accesses host resources.
+ *
+ * Note that this function does not call qemuDomainDetermineDiskChain as that 
is
+ * needed in qemuProcessPrepareHostStorage to remove disks based on the startup
+ * policy, thus other callers need to call it explicitly.
+ */
+int
+qemuProcessPrepareHostStorageDisk(virDomainObj *vm,
+  virDomainDiskDef *disk)
+{
+if (qemuProcessPrepareHostStorageSourceChain(vm, disk->src) < 0)
+return -1;
+
+return 0;
+}
+
+
 static int
 qemuProcessPrepareHostStorage(virQEMUDriver *driver,
   virDomainObj *vm,
@@ -6813,16 +6876,11 @@ qemuProcessPrepareHostStorage(virQEMUDriver *driver,
 return -1;
 }

-/* connect to any necessary vdpa block devices */
-for (i = vm->def->ndisks; i > 0; i--) {
-size_t idx = i - 1;
-virDomainDiskDef *disk = vm->def->disks[idx];
-virStorageSource *src;
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDef *disk = vm->def->disks[i];

-for (src = disk->src; virStorageSourceIsBacking(src); src = 
src->backingStore) {
-if (qemuProcessPrepareHostStorageSourceVDPA(src, vm->privateData) 
< 0)
-return -1;
-}
+if (qemuProcessPrepareHostStorageDisk(vm, disk) < 0)
+return -1;
 }

 return 0;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index ef05b46892..c1ea949215 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -133,6 +133,13 @@ int qemuProcessPrepareHost(virQEMUDriver *driver,
virDomainObj *vm,
unsigned int flags);

+int qemuProcessPrepareHostStorageSource(virDomainObj *vm,
+virStorageSource *src);
+int qemuProcessPrepareHostStorageSourceChain(virDomainObj *vm,
+ virStorageSource *chain);
+int qemuProcessPrepareHostStorageDisk(virDomainObj *vm,
+  virDomainDiskDef *disk);
+
 int qemuProcessDeleteThreadContext(virDomainObj *vm);

 int qemuProcessLaunch(virConnectPtr conn,
-- 
2.41.0



[PATCH v2 2/3] qemu: hotplug: Setup host side of VDPA device for disk hotplug

2023-10-26 Thread Peter Krempa
The code which opens the VDPA device and prepares it for FD passing was
not called in the hotplug code path, preventing hotplug of VDPA disks
with:

 error: internal error: argument key 'path' must not have null value

Use the new helper qemuProcessPrepareHostStorageDisk to setup the VDPA
definition.

Closes: https://gitlab.com/libvirt/libvirt/-/issues/539
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1f7f5bdd26..fec7c4be4e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1007,6 +1007,9 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver 
*driver,
 if (qemuDomainDetermineDiskChain(driver, vm, disk, NULL) < 0)
 goto cleanup;

+if (qemuProcessPrepareHostStorageDisk(vm, disk) < 0)
+goto cleanup;
+
 if (qemuHotplugAttachManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE) < 0)
 goto cleanup;

-- 
2.41.0



[PATCH v2 0/3] qemu: Fix hotplug and blockcopy to VDPA disks

2023-10-26 Thread Peter Krempa
Content-type: text/plain

v2:
- also deal with blockcopy (requires more helpers)


Peter Krempa (3):
  qemu: process: Extract host setup of disk device into helpers
  qemu: hotplug: Setup host side of VDPA device for disk hotplug
  qemu: Setup host side of VDPA device for block copy

 src/qemu/qemu_block.c   |  4 +++
 src/qemu/qemu_driver.c  | 12 +++
 src/qemu/qemu_hotplug.c |  3 ++
 src/qemu/qemu_process.c | 76 -
 src/qemu/qemu_process.h |  7 
 5 files changed, 93 insertions(+), 9 deletions(-)

-- 
2.41.0



Re: [PATCH 00/22] qemu: Refactor blockdev protocol JSON generators ('raw' driver removal part 2)

2023-10-26 Thread Ján Tomko

On a Wednesday in 2023, Peter Krempa wrote:

Second part cleans up tests and various bits and refactors the protocol
blockdev JSON configuration generator to be reusable.

This still doesn't change the configuration of the blockdevs, just
reorders some arguments.

Peter Krempa (22):
 qemuxml2(argv|xml)test: Add network backed disk type='sd'
 qemu: migration: No longer avoid 'auto-read-only' option for migration
 qemuBuildDriveSourceStr: Absorb only use of qemuDiskSourceGetProps
 qemuBlockStorageSourceGetBackendProps: Remove unnecessary indent for
   non-nbdkit code path
 conf: Move definition of some disk type enums to a common header
 virStorageSource: Use proper type for shadow copies of
   iomode/cachemode/discard/detect_zeroes
 qemuDomainDiskCachemodeFlags: Simplify usage
 qemuBlockStorageSourceGetBlockdevGetCacheProps: Return the cache
   object rather than appending it
 qemublocktest: Use "target only" mode in 'testJSONtoJSON' and
   'testBackingXMLjsonXML'
 qemublocktest: Drop 'sheepdog' and 'vxhs' test cases
 qemublocktest: Fix logical bug in TEST_JSON_FORMAT macro
 qemublocktest: testBackingXMLjsonXML: Drop 'legacy' mode
 qemuBlockStorageSourceGetBackendProps: Unify cases for '!onlytarget'
   and '!legacy'
 virDomainDiskGetDetectZeroesMode: Return proper type
 qemuBlockStorageSourceGetBackendProps: Unify ordering of fields
 qemu: block: Add helper to add common properties for -blockdev
   configuration
 qemu: block: Use qemuBlockStorageSourceAddBlockdevCommonProps for
   storage slice
 qemuBlockStorageSourceGetBackendProps: Introduce
   QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_EFFECTIVE_NODE
 qemuBuildHostdevSCSIAttachPrepare: Use "effective node" mode for
   getting blockdev props
 qemuBlockStorageSourceGetBackendProps: Use
   qemuBlockStorageSourceAddBlockdevCommonProps
 qemuBlockStorageSourceGetBackendProps: Remove unused logic for
   (auto-)read-only flags
 qemu: block: Remove unused flags
   QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_ flags

src/conf/domain_conf.c|   2 +-
src/conf/domain_conf.h|  39 +-
src/conf/storage_source_conf.h|   8 +-
src/conf/virconftypes.h   |  39 +-
src/qemu/qemu_block.c | 349 +-
src/qemu/qemu_block.h |   6 +-
src/qemu/qemu_command.c   |  63 ++--
src/qemu/qemu_domain.c|  33 +-
src/qemu/qemu_domain.h|   4 +-
src/qemu/qemu_migration.c |   5 +-
tests/qemublocktest.c |  41 +-
.../jsontojson/curl-libguestfs-out.json   |   5 +-
.../ssh-passthrough-libguestfs-out.json   |   4 +-
.../xml2json/dir-fat-cache.json   |   6 +-
.../file-backing_basic-cache-directsync.json  |  24 +-
.../file-backing_basic-cache-none.json|  24 +-
.../file-backing_basic-cache-unsafe.json  |  24 +-
.../file-backing_basic-cache-writeback.json   |  24 +-
...file-backing_basic-cache-writethrough.json |  24 +-
.../xml2json/file-raw-aio_native.json |   6 +-
...work-qcow2-backing-chain-cache-unsafe.json |  12 +-
.../blkdeviotune-group-num.x86_64-latest.args |   4 +-
...blkdeviotune-max-length.x86_64-latest.args |   4 +-
.../blkdeviotune-max.x86_64-latest.args   |   4 +-
.../blkdeviotune.x86_64-latest.args   |   4 +-
.../controller-order.x86_64-latest.args   |   2 +-
.../disk-aio.x86_64-latest.args   |   2 +-
.../disk-arm-virtio-sd.aarch64-latest.args|   2 +
tests/qemuxml2argvdata/disk-arm-virtio-sd.xml |  16 +
.../disk-cache.x86_64-latest.args |  10 +-
.../disk-error-policy-s390x.s390x-latest.args |   6 +-
.../disk-error-policy.x86_64-latest.args  |   6 +-
.../disk-metadata-cache.x86_64-latest.args|   6 +-
.../disk-network-nfs.x86_64-latest.args   |   2 +-
...rk-tlsx509-nbd-hostname.x86_64-latest.args |   2 +-
...disk-network-tlsx509-nbd.x86_64-5.2.0.args |   2 +-
...isk-network-tlsx509-nbd.x86_64-latest.args |   2 +-
...isk-network-tlsx509-vxhs.x86_64-5.0.0.args |   6 +-
.../disk-nvme.x86_64-latest.args  |   2 +-
.../disk-shared.x86_64-latest.args|   6 +-
.../disk-slices.x86_64-latest.args|   8 +-
.../disk-snapshot.x86_64-latest.args  |   4 +-
.../disk-transient.x86_64-latest.args |   2 +-
.../disk-vhostvdpa.x86_64-latest.args |   2 +-
.../name-escape.x86_64-latest.args|   2 +-
.../user-aliases.x86_64-latest.args   |   4 +-
...eo-bochs-display-device.x86_64-latest.args |   2 +-
...-device-pciaddr-default.x86_64-latest.args |   2 +-
...video-qxl-device-vgamem.x86_64-latest.args |   2 +-
.../video-qxl-device.x86_64-latest.args   |   2 +-
...o-qxl-sec-device-vgamem.x86_64-latest.args |   2 +-
.../video-qxl-sec-device.x86_64-latest.args   |   2 +-
...eo-ramfb-display-device.x86_64-latest.args |   2 +-
...video-vga-device-vgamem.x86_64-latest.args |   2 +-
.../video-vga-device.x86_6

Re: [PATCH 0/2] qemu: Fix hotplug of VDPA disks

2023-10-26 Thread Peter Krempa
On Thu, Oct 26, 2023 at 15:15:15 +0200, Peter Krempa wrote:
> Peter Krempa (2):
>   qemu: process: Extract host setup of disk device into a helper
>   qemu: hotplug: Setup host side of VDPA device for disk hotplug
> 
>  src/qemu/qemu_hotplug.c |  3 +++
>  src/qemu/qemu_process.c | 42 -
>  src/qemu/qemu_process.h |  3 +++
>  3 files changed, 39 insertions(+), 9 deletions(-)

I'll be posting a v2 to properly address also block copy.



[PATCH 1/2] qemu: process: Extract host setup of disk device into a helper

2023-10-26 Thread Peter Krempa
Currently the code sets up only VDPA backends but will be used later in
hotplug code too.

This patch also uses normal forward iteration in the loop in
qemuProcessPrepareHostStorage as we don't need to remove disks from the
disk list at that point.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_process.c | 42 -
 src/qemu/qemu_process.h |  3 +++
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 63c0c62a46..b420ec283d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6777,6 +6777,35 @@ qemuProcessPrepareHostStorageSourceVDPA(virStorageSource 
*src,
 }


+/**
+ * qemuProcessPrepareHostStorageDisk:
+ *
+ * @vm: domain object
+ * @disk: disk definition object
+ *
+ * Prepare the host side of a disk for use with the VM. Note that this function
+ * accesses host resources.
+ *
+ * Note that this function does not call qemuDomainDetermineDiskChain as that 
is
+ * needed in qemuProcessPrepareHostStorage to remove disks based on the startup
+ * policy, thus other callers need to call it explicitly.
+ */
+int
+qemuProcessPrepareHostStorageDisk(virDomainObj *vm,
+  virDomainDiskDef *disk)
+{
+virStorageSource *n;
+
+for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
+/* connect to any necessary vdpa block devices */
+if (qemuProcessPrepareHostStorageSourceVDPA(n, vm->privateData) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 qemuProcessPrepareHostStorage(virQEMUDriver *driver,
   virDomainObj *vm,
@@ -6813,16 +6842,11 @@ qemuProcessPrepareHostStorage(virQEMUDriver *driver,
 return -1;
 }

-/* connect to any necessary vdpa block devices */
-for (i = vm->def->ndisks; i > 0; i--) {
-size_t idx = i - 1;
-virDomainDiskDef *disk = vm->def->disks[idx];
-virStorageSource *src;
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDef *disk = vm->def->disks[i];

-for (src = disk->src; virStorageSourceIsBacking(src); src = 
src->backingStore) {
-if (qemuProcessPrepareHostStorageSourceVDPA(src, vm->privateData) 
< 0)
-return -1;
-}
+if (qemuProcessPrepareHostStorageDisk(vm, disk) < 0)
+return -1;
 }

 return 0;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index ef05b46892..7b974259d3 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -133,6 +133,9 @@ int qemuProcessPrepareHost(virQEMUDriver *driver,
virDomainObj *vm,
unsigned int flags);

+int qemuProcessPrepareHostStorageDisk(virDomainObj *vm,
+  virDomainDiskDef *disk);
+
 int qemuProcessDeleteThreadContext(virDomainObj *vm);

 int qemuProcessLaunch(virConnectPtr conn,
-- 
2.41.0



[PATCH 2/2] qemu: hotplug: Setup host side of VDPA device for disk hotplug

2023-10-26 Thread Peter Krempa
The code which opens the VDPA device and prepares it for FD passing was
not called in the hotplug code path, preventing hotplug of VDPA disks
with:

 error: internal error: argument key 'path' must not have null value

Use the new helper qemuProcessPrepareHostStorageDisk to setup the VDPA
definition.

Closes: https://gitlab.com/libvirt/libvirt/-/issues/539
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1f7f5bdd26..fec7c4be4e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1007,6 +1007,9 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver 
*driver,
 if (qemuDomainDetermineDiskChain(driver, vm, disk, NULL) < 0)
 goto cleanup;

+if (qemuProcessPrepareHostStorageDisk(vm, disk) < 0)
+goto cleanup;
+
 if (qemuHotplugAttachManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE) < 0)
 goto cleanup;

-- 
2.41.0



[PATCH 0/2] qemu: Fix hotplug of VDPA disks

2023-10-26 Thread Peter Krempa
Peter Krempa (2):
  qemu: process: Extract host setup of disk device into a helper
  qemu: hotplug: Setup host side of VDPA device for disk hotplug

 src/qemu/qemu_hotplug.c |  3 +++
 src/qemu/qemu_process.c | 42 -
 src/qemu/qemu_process.h |  3 +++
 3 files changed, 39 insertions(+), 9 deletions(-)

-- 
2.41.0



Re: [PATCH] docs: formatdomain: Clarify that the SLIC ACPI table config is available for all modes

2023-10-26 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Move the docs for the  element under a common section as
it's not specific for direct kernel boot. In fact the original use was
for Windows activation.

Fixes: 72f652da63255c7f1a9914625cce617dde9128d0
Signed-off-by: Peter Krempa 
---
docs/formatdomain.rst | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


[PATCH] docs: formatdomain: Clarify that the SLIC ACPI table config is available for all modes

2023-10-26 Thread Peter Krempa
Move the docs for the  element under a common section as
it's not specific for direct kernel boot. In fact the original use was
for Windows activation.

Fixes: 72f652da63255c7f1a9914625cce617dde9128d0
Signed-off-by: Peter Krempa 
---
 docs/formatdomain.rst | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 03735e4593..459815d2b5 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -387,9 +387,6 @@ and full virtualized guests.
  /root/f8-i386-initrd
  console=ttyS0 ks=http://example.com/f8-i386/os/
  /root/ppc.dtb
- 
-   /path/to/slic.dat
- 

...

@@ -413,11 +410,6 @@ and full virtualized guests.
The contents of this element specify the fully-qualified path to the
(optional) device tree binary (dtb) image in the host OS. :since:`Since
1.0.4`
-``acpi``
-   The ``table`` element contains a fully-qualified path to the ACPI table. The
-   ``type`` attribute contains the ACPI table type (currently only ``slic`` is
-   supported) :since:`Since 1.3.5 (QEMU)` :since:`Since 5.9.0 (Xen)`
-

 Container boot
 ~~
@@ -470,6 +462,27 @@ If you want to enable user namespace, set the ``idmap`` 
element. The ``uid`` and
  


+Common  element configuration
+~
+
+These options apply to any form of booting of the guest OS.
+
+::
+
+   ...
+   
+ ...
+ 
+   /path/to/slic.dat
+ 
+   
+   ...
+
+``acpi``
+   The ``table`` element contains a fully-qualified path to the ACPI table. The
+   ``type`` attribute contains the ACPI table type (currently only ``slic`` is
+   supported) :since:`Since 1.3.5 (QEMU)` :since:`Since 5.9.0 (Xen)`
+

 SMBIOS System Information
 -
-- 
2.41.0



Entering freeze for libvirt-9.9.0

2023-10-26 Thread Jiri Denemark
I have just tagged v9.9.0-rc1 in the repository and pushed signed
tarballs and source RPMs to https://download.libvirt.org/

Please give the release candidate some testing and in case you find a
serious issue which should have a fix in the upcoming release, feel
free to reply to this thread to make sure the issue is more visible.

If you have not done so yet, please update NEWS.rst to document any
significant change you made since the last release.

Thanks,

Jirka



Re: [libvirt PATCH v2 0/8] meson: Improve handling of tests

2023-10-26 Thread Martin Kletzander

On Wed, Oct 25, 2023 at 07:06:53PM +0200, Andrea Bolognani wrote:

Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1049326523

Changes from [v1]

 * fix test suite on macOS and ensure it is run as part of the
   pipeline (with the previous version it would just always fail);

 * disable a couple of tests (check-html, check-html-references)
   that I had missed the first time around.

[v1] https://listman.redhat.com/archives/libvir-list/2023-October/242491.html

Andrea Bolognani (8):
 tests: Fix some test cases on macOS
 ci: Disable optimizations on macOS
 meson: Do less when not building from git
 meson: Move all handling of test options together
 meson: Handle -Dtests=enabled with Clang
 meson: Make -Dexpensive_tests depend on -Dtests
 meson: Disable all tests when tests are disabled
 meson: Rename build_tests -> tests_enabled


Reviewed-by: Martin Kletzander 



build-aux/meson.build |  99 +
ci/cirrus/build.yml   |   2 +-
docs/html/meson.build |  20 +-
docs/meson.build  |  24 ++-
meson.build   |  75 ---
meson_options.txt |   2 +-
src/access/meson.build|  16 +-
src/meson.build   | 204 +-
.../disk-source-fd.x86_64-latest.args |  10 +-
.../disk-vhostvdpa.x86_64-latest.args |   2 +-
tests/qemuxml2argvtest.c  |   8 +-
11 files changed, 242 insertions(+), 220 deletions(-)

--
2.41.0



signature.asc
Description: PGP signature