[PATCH] hw/ufs: Fix incorrect register fields

2023-10-09 Thread Jeuk Kim
From: Jeuk Kim 

This patch fixes invalid ufs register fields.
This fixes an issue reported by Bin Meng that
caused ufs to fail over riscv.

Signed-off-by: Jeuk Kim 
---
 include/block/ufs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/block/ufs.h b/include/block/ufs.h
index fd884eb8ce..7631a5af10 100644
--- a/include/block/ufs.h
+++ b/include/block/ufs.h
@@ -111,14 +111,14 @@ REG32(UECT, offsetof(UfsReg, uect))
 REG32(UECDME, offsetof(UfsReg, uecdme))
 REG32(UTRIACR, offsetof(UfsReg, utriacr))
 REG32(UTRLBA, offsetof(UfsReg, utrlba))
-FIELD(UTRLBA, UTRLBA, 9, 22)
+FIELD(UTRLBA, UTRLBA, 10, 22)
 REG32(UTRLBAU, offsetof(UfsReg, utrlbau))
 REG32(UTRLDBR, offsetof(UfsReg, utrldbr))
 REG32(UTRLCLR, offsetof(UfsReg, utrlclr))
 REG32(UTRLRSR, offsetof(UfsReg, utrlrsr))
 REG32(UTRLCNR, offsetof(UfsReg, utrlcnr))
 REG32(UTMRLBA, offsetof(UfsReg, utmrlba))
-FIELD(UTMRLBA, UTMRLBA, 9, 22)
+FIELD(UTMRLBA, UTMRLBA, 10, 22)
 REG32(UTMRLBAU, offsetof(UfsReg, utmrlbau))
 REG32(UTMRLDBR, offsetof(UfsReg, utmrldbr))
 REG32(UTMRLCLR, offsetof(UfsReg, utmrlclr))
-- 
2.34.1




Re: [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base

2023-10-09 Thread Alex Bennée


Manos Pitsidianakis  writes:

> On Mon, 09 Oct 2023 12:59, Alex Bennée  wrote:
>>diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
>>index 51c3f97c2d..d0b963199c 100644
>>--- a/hw/virtio/meson.build
>>+++ b/hw/virtio/meson.build
>>@@ -18,8 +18,15 @@ if have_vhost
>> # fixme - this really should be generic
>> specific_virtio_ss.add(files('vhost-user.c'))
>> system_virtio_ss.add(files('vhost-user-base.c'))
>>+
>>+# MMIO Stubs
>> system_virtio_ss.add(files('vhost-user-device.c'))
>>+system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: 
>>files('vhost-user-rng.c'))
>>+
>>+# PCI Stubs
>> system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: 
>> files('vhost-user-device-pci.c'))
>>+system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 
>>'CONFIG_VHOST_USER_RNG'],
>>+ if_true: files('vhost-user-rng-pci.c'))
>
> Is there a reason why the target was moved to system_virtio_ss from
> virtio_pci_ss?

So we build it once, virtio_pci_ss is still:

  specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)

which means we build it once for every target which is overkill.

>
>>   endif
>>   if have_vhost_vdpa
>> system_virtio_ss.add(files('vhost-vdpa.c'))
>>@@ -34,10 +41,8 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', 
>>if_true: files('vhost-user-
>> specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: 
>> files('virtio-pmem.c'))
>> specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: 
>> files('vhost-vsock.c'))
>> specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: 
>> files('vhost-user-vsock.c'))
>>-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: 
>>files('virtio-rng.c'))
>
> Was this accidental? It's not added anywhere else, only deleted.

Oops yes. Didn't mean to delete the in-tree emulation. Will fix.

>
>> @@ -57,7 +61,6 @@ virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_FS',
>> if_true: files('vhost-user-fs-pc
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: 
>> files('virtio-crypto-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT_HOST', if_true: 
>> files('virtio-input-host-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: 
>> files('virtio-input-pci.c'))
>>-virtio_pci_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: 
>>files('virtio-rng-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: 
>> files('virtio-balloon-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: 
>> files('virtio-9p-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: 
>> files('virtio-scsi-pci.c'))
>
> Same here
>
> Manos


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro



Re: [PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices

2023-10-09 Thread Manos Pitsidianakis

On Mon, 09 Oct 2023 12:59, Alex Bennée  wrote:
diff --git a/docs/system/devices/vhost-user.rst 
b/docs/system/devices/vhost-user.rst

index a80e95a48a..0f9eec3f00 100644
--- a/docs/system/devices/vhost-user.rst
+++ b/docs/system/devices/vhost-user.rst
@@ -15,6 +15,47 @@ to the guest. The code is mostly boilerplate although each 
device has
a ``chardev`` option which specifies the ID of the ``--chardev``
device that connects via a socket to the vhost-user *daemon*.

+Each device will have an virtio-mmio and virtio-pci variant. See your
+platform details for what sort of virtio bus to use.
+
+.. list-table:: vhost-user devices
+  :widths: 20 20 60
+  :header-rows: 1
+
+  * - Device
+- Type
+- Notes
+  * - vhost-user-device
+- Generic Development Device
+- You must manually specify ``virtio-id`` and the correct ``num_vqs``. 
Intended for expert use.


May be worth specifying they are `VHostUserBase` interface fields since 
it's not directly obvious if you come across this before reading the 
vhost-user-device code.



+  * - vhost-user-blk
+- Block storage
+-
+  * - vhost-user-fs
+- File based storage driver
+- See https://gitlab.com/virtio-fs/virtiofsd
+  * - vhost-user-scsi
+- SCSI based storage
+- See contrib/vhost-user/scsi
+  * - vhost-user-gpio
+- Proxy gpio pins to host
+- See https://github.com/rust-vmm/vhost-device
+  * - vhost-user-i2c
+- Proxy i2c devices to host
+- See https://github.com/rust-vmm/vhost-device
+  * - vhost-user-input
+- Generic input driver
+- See contrib/vhost-user-input
+  * - vhost-user-rng
+- Entropy driver
+- :ref:`vhost_user_rng`
+  * - vhost-user-gpu
+- GPU driver
+-
+  * - vhost-user-vsock
+- Socket based communication
+- See https://github.com/rust-vmm/vhost-device
+


There's also:

- hw/virtio/vhost-user-scmi.c
- hw/virtio/vhost-user-snd.c



Re: [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base

2023-10-09 Thread Manos Pitsidianakis

On Mon, 09 Oct 2023 12:59, Alex Bennée  wrote:

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 51c3f97c2d..d0b963199c 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -18,8 +18,15 @@ if have_vhost
# fixme - this really should be generic
specific_virtio_ss.add(files('vhost-user.c'))
system_virtio_ss.add(files('vhost-user-base.c'))
+
+# MMIO Stubs
system_virtio_ss.add(files('vhost-user-device.c'))
+system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: 
files('vhost-user-rng.c'))
+
+# PCI Stubs
system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: 
files('vhost-user-device-pci.c'))
+system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_RNG'],
+ if_true: files('vhost-user-rng-pci.c'))


Is there a reason why the target was moved to system_virtio_ss from 
virtio_pci_ss?



  endif
  if have_vhost_vdpa
system_virtio_ss.add(files('vhost-vdpa.c'))
@@ -34,10 +41,8 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', 
if_true: files('vhost-user-
specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: 
files('virtio-pmem.c'))
specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: 
files('vhost-vsock.c'))
specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: 
files('vhost-user-vsock.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: 
files('virtio-rng.c'))


Was this accidental? It's not added anywhere else, only deleted.

@@ -57,7 +61,6 @@ virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_FS', 
if_true: files('vhost-user-fs-pc

virtio_pci_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: 
files('virtio-crypto-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT_HOST', if_true: 
files('virtio-input-host-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: 
files('virtio-input-pci.c'))
-virtio_pci_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: 
files('virtio-rng-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: 
files('virtio-balloon-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-pci.c'))
virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: 
files('virtio-scsi-pci.c'))


Same here

Manos



Re: [PATCH v4 4/6] hw/virtio: derive vhost-user-i2c from vhost-user-base

2023-10-09 Thread Viresh Kumar
On 09-10-23, 10:59, Alex Bennée wrote:
> Now we can take advantage of the new base class and make
> vhost-user-i2c a much simpler boilerplate wrapper. Also as this
> doesn't require any target specific hacks we only need to build the
> stubs once.
> 
> Message-Id: <20230418162140.373219-13-alex.ben...@linaro.org>
> Acked-by: Mark Cave-Ayland 
> Signed-off-by: Alex Bennée 
> 
> ---
> v2
>   - update to new inheritance scheme
>   - move build to common code
> v3
>   - fix merge conflict in meson
>   - style updates, remove duplicate includes
> v4
>   - use vqsize
> ---
>  include/hw/virtio/vhost-user-i2c.h |  14 +-
>  hw/virtio/vhost-user-i2c.c | 272 ++---
>  hw/virtio/meson.build  |   5 +-
>  3 files changed, 23 insertions(+), 268 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh



Re: [PATCH v4 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base

2023-10-09 Thread Viresh Kumar
On 09-10-23, 10:59, Alex Bennée wrote:
> Now the new base class supports config handling we can take advantage
> and make vhost-user-gpio a much simpler boilerplate wrapper. Also as
> this doesn't require any target specific hacks we only need to build
> the stubs once.
> 
> Message-Id: <20230418162140.373219-12-alex.ben...@linaro.org>
> Signed-off-by: Alex Bennée 
> Acked-by: Mark Cave-Ayland 
> 
> ---
> v2
>   - use new vhost-user-base
>   - move build to common code
> v3
>   - fix inadvertent double link
> v4
>   - merge conflict
>   - update includes
> ---
>  include/hw/virtio/vhost-user-gpio.h |  23 +-
>  hw/virtio/vhost-user-gpio.c | 407 ++--
>  hw/virtio/meson.build   |   5 +-
>  3 files changed, 22 insertions(+), 413 deletions(-)

Looks nice. Thanks.

Acked-by: Viresh Kumar 

-- 
viresh



Re: [PATCH 02/10] tests/throttle: Clean up global variable shadowing

2023-10-09 Thread Alberto Garcia
On Mon 09 Oct 2023 12:02:43 PM +02, Philippe Mathieu-Daudé wrote:
> Follow all other tests pattern from this file, use the
> global 'cfg' variable to fix:
>
>   tests/unit/test-throttle.c:621:20: error: declaration shadows a variable in 
> the global scope [-Werror,-Wshadow]
>   ThrottleConfig cfg;
>  ^
>   tests/unit/test-throttle.c:28:23: note: previous declaration is here
>   static ThrottleConfig cfg;
> ^
>
> Signed-off-by: Philippe Mathieu-Daudé 

Acked-by: Alberto Garcia 

Berto



Re: [PATCH 05/10] tests/hd-geo-test: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé

On 9/10/23 12:02, Philippe Mathieu-Daudé wrote:

Rename the variable to fix:

   tests/qtest/hd-geo-test.c:610:11: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
   char *img_file_name;
 ^
   tests/qtest/hd-geo-test.c:80:14: note: previous declaration is here
   static char *img_file_name[backend_last];
^

Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/qtest/hd-geo-test.c | 12 
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index d08bffad91..e5e28c412d 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -78,6 +78,7 @@ static const CHST hd_chst[backend_last][mbr_last] = {
  };
  
  static char *img_file_name[backend_last];

+static char *qcow2_imgpath;
  
  static const CHST *cur_ide[4];
  
@@ -607,18 +608,17 @@ static TestArgs *create_args(void)

  static void add_drive_with_mbr(TestArgs *args,
 MBRpartitions mbr, uint64_t sectors)
  {
-char *img_file_name;
  char part[300];
  int ret;
  
  g_assert(args->n_drives < MAX_DRIVES);
  
-img_file_name = create_qcow2_with_mbr(mbr, sectors);

+qcow2_imgpath = create_qcow2_with_mbr(mbr, sectors);
  
-args->drives[args->n_drives] = img_file_name;

+args->drives[args->n_drives] = qcow2_imgpath;
  ret = snprintf(part, sizeof(part),
 "-drive file=%s,if=none,format=qcow2,id=disk%d",
-   img_file_name, args->n_drives);
+   qcow2_imgpath, args->n_drives);
  g_assert((0 < ret) && (ret <= sizeof(part)));
  args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, 
g_strdup(part));
  args->n_drives++;
@@ -1139,6 +1139,10 @@ test_add_done:
  g_free(img_file_name[i]);
  }
  }
+if (qcow2_imgpath) {
+unlink(qcow2_imgpath);
+g_free(qcow2_imgpath);


Oops I squashed this part while rebasing, this was supposed to
be a different patch.


+}
  
  return ret;

  }





[PATCH 06/10] tests/rtl8139: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the variable to fix:

  tests/qtest/rtl8139-test.c:28:33: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
  static void save_fn(QPCIDevice *dev, int devfn, void *data)
  ^
  tests/qtest/rtl8139-test.c:37:17: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
  QPCIDevice *dev;
  ^
  tests/qtest/rtl8139-test.c:25:20: note: previous declaration is here
  static QPCIDevice *dev;
 ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/rtl8139-test.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/rtl8139-test.c b/tests/qtest/rtl8139-test.c
index 4dc0a0d22e..eedf90f65a 100644
--- a/tests/qtest/rtl8139-test.c
+++ b/tests/qtest/rtl8139-test.c
@@ -22,7 +22,7 @@ static void nop(void)
 #define CLK 
 
 static QPCIBus *pcibus;
-static QPCIDevice *dev;
+static QPCIDevice *pcidev;
 static QPCIBar dev_bar;
 
 static void save_fn(QPCIDevice *dev, int devfn, void *data)
@@ -46,7 +46,7 @@ static QPCIDevice *get_device(void)
 #define PORT(name, len, val) \
 static unsigned __attribute__((unused)) in_##name(void) \
 { \
-unsigned res = qpci_io_read##len(dev, dev_bar, (val)); \
+unsigned res = qpci_io_read##len(pcidev, dev_bar, (val)); \
 if (verbosity_level >= 2) { \
 g_test_message("*%s -> %x", #name, res); \
 } \
@@ -57,7 +57,7 @@ static void out_##name(unsigned v) \
 if (verbosity_level >= 2) { \
 g_test_message("%x -> *%s", v, #name); \
 } \
-qpci_io_write##len(dev, dev_bar, (val), v);\
+qpci_io_write##len(pcidev, dev_bar, (val), v);\
 }
 
 PORT(Timer, l, 0x48)
@@ -189,11 +189,11 @@ static void test_init(void)
 {
 uint64_t barsize;
 
-dev = get_device();
+pcidev = get_device();
 
-dev_bar = qpci_iomap(dev, 0, );
+dev_bar = qpci_iomap(pcidev, 0, );
 
-qpci_device_enable(dev);
+qpci_device_enable(pcidev);
 
 test_timer();
 }
-- 
2.41.0




[PATCH 07/10] tests/npcm7xx_adc: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the global 'adc' variable in order to avoid:

  tests/qtest/npcm7xx_adc-test.c:98:58: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static uint32_t adc_read_con(QTestState *qts, const ADC *adc)
   ^
  tests/qtest/npcm7xx_adc-test.c:103:55: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static void adc_write_con(QTestState *qts, const ADC *adc, uint32_t value)
^
  tests/qtest/npcm7xx_adc-test.c:108:59: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static uint32_t adc_read_data(QTestState *qts, const ADC *adc)
^
  tests/qtest/npcm7xx_adc-test.c:119:53: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static void adc_qom_set(QTestState *qts, const ADC *adc,
  ^
  tests/qtest/npcm7xx_adc-test.c:135:57: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static void adc_write_input(QTestState *qts, const ADC *adc,
  ^
  tests/qtest/npcm7xx_adc-test.c:144:56: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static void adc_write_vref(QTestState *qts, const ADC *adc, uint32_t value)
 ^
  tests/qtest/npcm7xx_adc-test.c:162:59: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static uint32_t adc_prescaler(QTestState *qts, const ADC *adc)
^
  tests/qtest/npcm7xx_adc-test.c:175:64: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static void adc_wait_conv_finished(QTestState *qts, const ADC *adc,
 ^
  tests/qtest/npcm7xx_adc-test.c:196:16: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
const ADC *adc = adc_p;
   ^
  tests/qtest/npcm7xx_adc-test.c:207:16: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
const ADC *adc = adc_p;
   ^
  tests/qtest/npcm7xx_adc-test.c:235:16: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
const ADC *adc = adc_p;
   ^
  tests/qtest/npcm7xx_adc-test.c:267:16: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
const ADC *adc = adc_p;
   ^
  tests/qtest/npcm7xx_adc-test.c:293:16: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
const ADC *adc = adc_p;
   ^
  tests/qtest/npcm7xx_adc-test.c:311:16: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
const ADC *adc = adc_p;
   ^
  tests/qtest/npcm7xx_adc-test.c:93:5: note: previous declaration is here
  ADC adc = {
  ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/npcm7xx_adc-test.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/npcm7xx_adc-test.c b/tests/qtest/npcm7xx_adc-test.c
index 8048044d28..e751a72e36 100644
--- a/tests/qtest/npcm7xx_adc-test.c
+++ b/tests/qtest/npcm7xx_adc-test.c
@@ -90,7 +90,7 @@ typedef struct ADC {
 uint64_t base_addr;
 } ADC;
 
-ADC adc = {
+ADC adc_defs = {
 .irq= 0,
 .base_addr  = 0xf000c000
 };
@@ -367,12 +367,12 @@ int main(int argc, char **argv)
 {
 g_test_init(, , NULL);
 
-add_test(init, );
-add_test(convert_internal, );
-add_test(convert_external, );
-add_test(interrupt, );
-add_test(reset, );
-add_test(calibrate, );
+add_test(init, _defs);
+add_test(convert_internal, _defs);
+add_test(convert_external, _defs);
+add_test(interrupt, _defs);
+add_test(reset, _defs);
+add_test(calibrate, _defs);
 
 return g_test_run();
 }
-- 
2.41.0




[PATCH 08/10] tests/aio: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the argument to fix:

  tests/unit/test-aio.c:130:44: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
  static void set_event_notifier(AioContext *ctx, EventNotifier *notifier,
 ^
  tests/unit/test-aio.c:22:20: note: previous declaration is here
  static AioContext *ctx;
 ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-aio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test-aio.c b/tests/unit/test-aio.c
index 71ed31a4db..337b6e4ea7 100644
--- a/tests/unit/test-aio.c
+++ b/tests/unit/test-aio.c
@@ -127,10 +127,10 @@ static void *test_acquire_thread(void *opaque)
 return NULL;
 }
 
-static void set_event_notifier(AioContext *ctx, EventNotifier *notifier,
+static void set_event_notifier(AioContext *nctx, EventNotifier *notifier,
EventNotifierHandler *handler)
 {
-aio_set_event_notifier(ctx, notifier, handler, NULL, NULL);
+aio_set_event_notifier(nctx, notifier, handler, NULL, NULL);
 }
 
 static void dummy_notifier_read(EventNotifier *n)
-- 
2.41.0




[PATCH 04/10] tests/cdrom-test: Clean up global variable shadowing in prepare_image()

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the variable to fix:

  tests/qtest/cdrom-test.c:40:50: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
  static int prepare_image(const char *arch, char *isoimage)
   ^
  tests/qtest/cdrom-test.c:18:13: note: previous declaration is here
  static char isoimage[] = "cdrom-boot-iso-XX";
  ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/cdrom-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index f2a8d91929..0945383789 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -37,17 +37,17 @@ static int exec_xorrisofs(const char **args)
 return exit_status;
 }
 
-static int prepare_image(const char *arch, char *isoimage)
+static int prepare_image(const char *arch, char *isoimagepath)
 {
 char srcdir[] = "cdrom-test-dir-XX";
 char *codefile = NULL;
 int ifh, ret = -1;
 const char *args[] = {
 "xorrisofs", "-quiet", "-l", "-no-emul-boot",
-"-b", NULL, "-o", isoimage, srcdir, NULL
+"-b", NULL, "-o", isoimagepath, srcdir, NULL
 };
 
-ifh = mkstemp(isoimage);
+ifh = mkstemp(isoimagepath);
 if (ifh < 0) {
 perror("Error creating temporary iso image file");
 return -1;
-- 
2.41.0




[PATCH 09/10] tests/aio-multithread: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the argument to avoid:

  tests/unit/test-aio-multithread.c:226:37: error: declaration shadows a 
variable in the global scope [-Werror,-Wshadow]
  static void test_multi_co_mutex(int threads, int seconds)
  ^
  tests/unit/test-aio-multithread.c:401:34: error: declaration shadows a 
variable in the global scope [-Werror,-Wshadow]
  static void test_multi_mutex(int threads, int seconds)
   ^
  tests/unit/test-aio-multithread.c:24:18: note: previous declaration is here
  static IOThread *threads[NUM_CONTEXTS];
   ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-aio-multithread.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/unit/test-aio-multithread.c 
b/tests/unit/test-aio-multithread.c
index 08d4570ccb..d587f20667 100644
--- a/tests/unit/test-aio-multithread.c
+++ b/tests/unit/test-aio-multithread.c
@@ -223,7 +223,7 @@ static void coroutine_fn test_multi_co_mutex_entry(void 
*opaque)
 qatomic_dec();
 }
 
-static void test_multi_co_mutex(int threads, int seconds)
+static void test_multi_co_mutex(unsigned ctx_num, int seconds)
 {
 int i;
 
@@ -233,9 +233,9 @@ static void test_multi_co_mutex(int threads, int seconds)
 now_stopping = false;
 
 create_aio_contexts();
-assert(threads <= NUM_CONTEXTS);
-running = threads;
-for (i = 0; i < threads; i++) {
+assert(ctx_num <= NUM_CONTEXTS);
+running = ctx_num;
+for (i = 0; i < ctx_num; i++) {
 Coroutine *co1 = qemu_coroutine_create(test_multi_co_mutex_entry, 
NULL);
 aio_co_schedule(ctx[i], co1);
 }
@@ -398,7 +398,7 @@ static void test_multi_mutex_entry(void *opaque)
 qatomic_dec();
 }
 
-static void test_multi_mutex(int threads, int seconds)
+static void test_multi_mutex(unsigned ctx_num, int seconds)
 {
 int i;
 
@@ -408,9 +408,9 @@ static void test_multi_mutex(int threads, int seconds)
 now_stopping = false;
 
 create_aio_contexts();
-assert(threads <= NUM_CONTEXTS);
-running = threads;
-for (i = 0; i < threads; i++) {
+assert(ctx_num <= NUM_CONTEXTS);
+running = ctx_num;
+for (i = 0; i < ctx_num; i++) {
 Coroutine *co1 = qemu_coroutine_create(test_multi_mutex_entry, NULL);
 aio_co_schedule(ctx[i], co1);
 }
-- 
2.41.0




[PATCH 01/10] system/qtest: Clean up global variable shadowing in qtest_server_init()

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the variable to fix:

  softmmu/qtest.c:869:13: error: declaration shadows a variable in the global 
scope [-Werror,-Wshadow]
  Object *qtest;
  ^
  softmmu/qtest.c:53:15: note: previous declaration is here
  static QTest *qtest;
^

Signed-off-by: Philippe Mathieu-Daudé 
---
 softmmu/qtest.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 35b643a274..7964f0b248 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -866,7 +866,7 @@ void qtest_server_init(const char *qtest_chrdev, const char 
*qtest_log, Error **
 {
 ERRP_GUARD();
 Chardev *chr;
-Object *qtest;
+Object *qobj;
 
 chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
 if (chr == NULL) {
@@ -875,18 +875,18 @@ void qtest_server_init(const char *qtest_chrdev, const 
char *qtest_log, Error **
 return;
 }
 
-qtest = object_new(TYPE_QTEST);
-object_property_set_str(qtest, "chardev", chr->label, _abort);
+qobj = object_new(TYPE_QTEST);
+object_property_set_str(qobj, "chardev", chr->label, _abort);
 if (qtest_log) {
-object_property_set_str(qtest, "log", qtest_log, _abort);
+object_property_set_str(qobj, "log", qtest_log, _abort);
 }
-object_property_add_child(qdev_get_machine(), "qtest", qtest);
-user_creatable_complete(USER_CREATABLE(qtest), errp);
+object_property_add_child(qdev_get_machine(), "qtest", qobj);
+user_creatable_complete(USER_CREATABLE(qobj), errp);
 if (*errp) {
-object_unparent(qtest);
+object_unparent(qobj);
 }
 object_unref(OBJECT(chr));
-object_unref(qtest);
+object_unref(qobj);
 }
 
 static bool qtest_server_start(QTest *q, Error **errp)
-- 
2.41.0




[PATCH 10/10] tests/coroutine: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the global variable to avoid:

  tests/unit/test-coroutine.c:430:11: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
  bool *done = opaque;
^
  tests/unit/test-coroutine.c:438:10: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
  bool done = false;
   ^
  tests/unit/test-coroutine.c:198:12: note: previous declaration is here
  static int done;
 ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-coroutine.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index b0d21d673a..29695cbdcb 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -195,7 +195,7 @@ static void test_no_dangling_access(void)
 }
 
 static bool locked;
-static int done;
+static int done_count;
 
 static void coroutine_fn mutex_fn(void *opaque)
 {
@@ -206,7 +206,7 @@ static void coroutine_fn mutex_fn(void *opaque)
 qemu_coroutine_yield();
 locked = false;
 qemu_co_mutex_unlock(m);
-done++;
+done_count++;
 }
 
 static void coroutine_fn lockable_fn(void *opaque)
@@ -218,7 +218,7 @@ static void coroutine_fn lockable_fn(void *opaque)
 qemu_coroutine_yield();
 locked = false;
 qemu_lockable_unlock(x);
-done++;
+done_count++;
 }
 
 static void do_test_co_mutex(CoroutineEntry *entry, void *opaque)
@@ -226,7 +226,7 @@ static void do_test_co_mutex(CoroutineEntry *entry, void 
*opaque)
 Coroutine *c1 = qemu_coroutine_create(entry, opaque);
 Coroutine *c2 = qemu_coroutine_create(entry, opaque);
 
-done = 0;
+done_count = 0;
 qemu_coroutine_enter(c1);
 g_assert(locked);
 qemu_coroutine_enter(c2);
@@ -235,11 +235,11 @@ static void do_test_co_mutex(CoroutineEntry *entry, void 
*opaque)
  * terminates.
  */
 qemu_coroutine_enter(c1);
-g_assert_cmpint(done, ==, 1);
+g_assert_cmpint(done_count, ==, 1);
 g_assert(locked);
 
 qemu_coroutine_enter(c2);
-g_assert_cmpint(done, ==, 2);
+g_assert_cmpint(done_count, ==, 2);
 g_assert(!locked);
 }
 
-- 
2.41.0




[PATCH 05/10] tests/hd-geo-test: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the variable to fix:

  tests/qtest/hd-geo-test.c:610:11: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
  char *img_file_name;
^
  tests/qtest/hd-geo-test.c:80:14: note: previous declaration is here
  static char *img_file_name[backend_last];
   ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/hd-geo-test.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index d08bffad91..e5e28c412d 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -78,6 +78,7 @@ static const CHST hd_chst[backend_last][mbr_last] = {
 };
 
 static char *img_file_name[backend_last];
+static char *qcow2_imgpath;
 
 static const CHST *cur_ide[4];
 
@@ -607,18 +608,17 @@ static TestArgs *create_args(void)
 static void add_drive_with_mbr(TestArgs *args,
MBRpartitions mbr, uint64_t sectors)
 {
-char *img_file_name;
 char part[300];
 int ret;
 
 g_assert(args->n_drives < MAX_DRIVES);
 
-img_file_name = create_qcow2_with_mbr(mbr, sectors);
+qcow2_imgpath = create_qcow2_with_mbr(mbr, sectors);
 
-args->drives[args->n_drives] = img_file_name;
+args->drives[args->n_drives] = qcow2_imgpath;
 ret = snprintf(part, sizeof(part),
"-drive file=%s,if=none,format=qcow2,id=disk%d",
-   img_file_name, args->n_drives);
+   qcow2_imgpath, args->n_drives);
 g_assert((0 < ret) && (ret <= sizeof(part)));
 args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, g_strdup(part));
 args->n_drives++;
@@ -1139,6 +1139,10 @@ test_add_done:
 g_free(img_file_name[i]);
 }
 }
+if (qcow2_imgpath) {
+unlink(qcow2_imgpath);
+g_free(qcow2_imgpath);
+}
 
 return ret;
 }
-- 
2.41.0




[PATCH 02/10] tests/throttle: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Follow all other tests pattern from this file, use the
global 'cfg' variable to fix:

  tests/unit/test-throttle.c:621:20: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
  ThrottleConfig cfg;
 ^
  tests/unit/test-throttle.c:28:23: note: previous declaration is here
  static ThrottleConfig cfg;
^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-throttle.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index ac35d65d19..2146cfacd3 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -618,7 +618,6 @@ static bool do_test_accounting(bool is_ops, /* are we 
testing bps or ops */
  { THROTTLE_OPS_TOTAL,
THROTTLE_OPS_READ,
THROTTLE_OPS_WRITE, } };
-ThrottleConfig cfg;
 BucketType index;
 int i;
 
-- 
2.41.0




[PATCH 03/10] tests/virtio-scsi: Clean up global variable shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Rename the (unused) 'allow' argument, following the pattern
used by the other tests in this file. This fixes:

  tests/qtest/virtio-scsi-test.c:159:61: error: declaration shadows a variable 
in the global scope [-Werror,-Wshadow]
  static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
  ^
  tests/qtest/virtio-scsi-test.c:37:25: note: previous declaration is here
  static QGuestAllocator *alloc;
  ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/virtio-scsi-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
index ceaa7f2415..db10d572d0 100644
--- a/tests/qtest/virtio-scsi-test.c
+++ b/tests/qtest/virtio-scsi-test.c
@@ -156,7 +156,7 @@ static QVirtioSCSIQueues *qvirtio_scsi_init(QVirtioDevice 
*dev)
 return vs;
 }
 
-static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
+static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
 {
 QTestState *qts = global_qtest;
 
-- 
2.41.0




[PATCH 00/10] tests: Clean up global variables shadowing

2023-10-09 Thread Philippe Mathieu-Daudé
Clean up global variables shadowing in tests/
in order to be able to use -Wshadow with Clang.

Philippe Mathieu-Daudé (10):
  system/qtest: Clean up global variable shadowing in
qtest_server_init()
  tests/throttle: Clean up global variable shadowing
  tests/virtio-scsi: Clean up global variable shadowing
  tests/cdrom-test: Clean up global variable shadowing in
prepare_image()
  tests/hd-geo-test: Clean up global variable shadowing
  tests/rtl8139: Clean up global variable shadowing
  tests/npcm7xx_adc: Clean up global variable shadowing
  tests/aio: Clean up global variable shadowing
  tests/aio-multithread: Clean up global variable shadowing
  tests/coroutine: Clean up global variable shadowing

 softmmu/qtest.c   | 16 
 tests/qtest/cdrom-test.c  |  6 +++---
 tests/qtest/hd-geo-test.c | 12 
 tests/qtest/npcm7xx_adc-test.c| 14 +++---
 tests/qtest/rtl8139-test.c| 12 ++--
 tests/qtest/virtio-scsi-test.c|  2 +-
 tests/unit/test-aio-multithread.c | 16 
 tests/unit/test-aio.c |  4 ++--
 tests/unit/test-coroutine.c   | 12 ++--
 tests/unit/test-throttle.c|  1 -
 10 files changed, 49 insertions(+), 46 deletions(-)

-- 
2.41.0




[PATCH v4 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base

2023-10-09 Thread Alex Bennée
Now the new base class supports config handling we can take advantage
and make vhost-user-gpio a much simpler boilerplate wrapper. Also as
this doesn't require any target specific hacks we only need to build
the stubs once.

Message-Id: <20230418162140.373219-12-alex.ben...@linaro.org>
Signed-off-by: Alex Bennée 
Acked-by: Mark Cave-Ayland 

---
v2
  - use new vhost-user-base
  - move build to common code
v3
  - fix inadvertent double link
v4
  - merge conflict
  - update includes
---
 include/hw/virtio/vhost-user-gpio.h |  23 +-
 hw/virtio/vhost-user-gpio.c | 407 ++--
 hw/virtio/meson.build   |   5 +-
 3 files changed, 22 insertions(+), 413 deletions(-)

diff --git a/include/hw/virtio/vhost-user-gpio.h 
b/include/hw/virtio/vhost-user-gpio.h
index a9d3f9b049..5201d5f072 100644
--- a/include/hw/virtio/vhost-user-gpio.h
+++ b/include/hw/virtio/vhost-user-gpio.h
@@ -12,33 +12,14 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-user.h"
-#include "standard-headers/linux/virtio_gpio.h"
-#include "chardev/char-fe.h"
+#include "hw/virtio/vhost-user-base.h"
 
 #define TYPE_VHOST_USER_GPIO "vhost-user-gpio-device"
 OBJECT_DECLARE_SIMPLE_TYPE(VHostUserGPIO, VHOST_USER_GPIO);
 
 struct VHostUserGPIO {
 /*< private >*/
-VirtIODevice parent_obj;
-CharBackend chardev;
-struct virtio_gpio_config config;
-struct vhost_virtqueue *vhost_vqs;
-struct vhost_dev vhost_dev;
-VhostUserState vhost_user;
-VirtQueue *command_vq;
-VirtQueue *interrupt_vq;
-/**
- * There are at least two steps of initialization of the
- * vhost-user device. The first is a "connect" step and
- * second is a "start" step. Make a separation between
- * those initialization phases by using two fields.
- *
- * @connected: see vu_gpio_connect()/vu_gpio_disconnect()
- * @started_vu: see vu_gpio_start()/vu_gpio_stop()
- */
-bool connected;
-bool started_vu;
+VHostUserBase parent;
 /*< public >*/
 };
 
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index 3d7fae3984..9f37c25415 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -11,388 +11,25 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/vhost-user-gpio.h"
-#include "qemu/error-report.h"
 #include "standard-headers/linux/virtio_ids.h"
-#include "trace.h"
+#include "standard-headers/linux/virtio_gpio.h"
 
-#define REALIZE_CONNECTION_RETRIES 3
-#define VHOST_NVQS 2
-
-/* Features required from VirtIO */
-static const int feature_bits[] = {
-VIRTIO_F_VERSION_1,
-VIRTIO_F_NOTIFY_ON_EMPTY,
-VIRTIO_RING_F_INDIRECT_DESC,
-VIRTIO_RING_F_EVENT_IDX,
-VIRTIO_GPIO_F_IRQ,
-VIRTIO_F_RING_RESET,
-VHOST_INVALID_FEATURE_BIT
-};
-
-static void vu_gpio_get_config(VirtIODevice *vdev, uint8_t *config)
-{
-VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-
-memcpy(config, >config, sizeof(gpio->config));
-}
-
-static int vu_gpio_config_notifier(struct vhost_dev *dev)
-{
-VHostUserGPIO *gpio = VHOST_USER_GPIO(dev->vdev);
-
-memcpy(dev->vdev->config, >config, sizeof(gpio->config));
-virtio_notify_config(dev->vdev);
-
-return 0;
-}
-
-const VhostDevConfigOps gpio_ops = {
-.vhost_dev_config_notifier = vu_gpio_config_notifier,
+static Property vgpio_properties[] = {
+DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+DEFINE_PROP_END_OF_LIST(),
 };
 
-static int vu_gpio_start(VirtIODevice *vdev)
-{
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-struct vhost_dev *vhost_dev = >vhost_dev;
-int ret, i;
-
-if (!k->set_guest_notifiers) {
-error_report("binding does not support guest notifiers");
-return -ENOSYS;
-}
-
-ret = vhost_dev_enable_notifiers(vhost_dev, vdev);
-if (ret < 0) {
-error_report("Error enabling host notifiers: %d", ret);
-return ret;
-}
-
-ret = k->set_guest_notifiers(qbus->parent, vhost_dev->nvqs, true);
-if (ret < 0) {
-error_report("Error binding guest notifier: %d", ret);
-goto err_host_notifiers;
-}
-
-/*
- * Before we start up we need to ensure we have the final feature
- * set needed for the vhost configuration. The backend may also
- * apply backend_features when the feature set is sent.
- */
-vhost_ack_features(>vhost_dev, feature_bits, vdev->guest_features);
-
-ret = vhost_dev_start(>vhost_dev, vdev, false);
-if (ret < 0) {
-error_report("Error starting vhost-user-gpio: %d", ret);
-goto err_guest_notifiers;
-}
-gpio->started_vu = true;
-
-/*
- * guest_notifier_mask/pending not used yet, so just unmask
- * everything here. virtio-pci will do the right thing by
- * enabling/disabling irqfd.
- */
-for (i = 0; i 

[PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base

2023-10-09 Thread Alex Bennée
Now we can take advantage of our new base class and make
vhost-user-rng a much simpler boilerplate wrapper. Also as this
doesn't require any target specific hacks we only need to build the
stubs once.

Message-Id: <20230418162140.373219-10-alex.ben...@linaro.org>
Acked-by: Mark Cave-Ayland 
Signed-off-by: Alex Bennée 

---
v2
  - new derivation layout
  - move directly to softmmu_virtio_ss
v3
  - use vqsize
---
 include/hw/virtio/vhost-user-rng.h |  11 +-
 hw/virtio/vhost-user-rng.c | 278 +++--
 hw/virtio/meson.build  |  11 +-
 3 files changed, 31 insertions(+), 269 deletions(-)

diff --git a/include/hw/virtio/vhost-user-rng.h 
b/include/hw/virtio/vhost-user-rng.h
index ddd9f01eea..6cffe28807 100644
--- a/include/hw/virtio/vhost-user-rng.h
+++ b/include/hw/virtio/vhost-user-rng.h
@@ -12,21 +12,14 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-user.h"
-#include "chardev/char-fe.h"
+#include "hw/virtio/vhost-user-base.h"
 
 #define TYPE_VHOST_USER_RNG "vhost-user-rng"
 OBJECT_DECLARE_SIMPLE_TYPE(VHostUserRNG, VHOST_USER_RNG)
 
 struct VHostUserRNG {
 /*< private >*/
-VirtIODevice parent;
-CharBackend chardev;
-struct vhost_virtqueue *vhost_vq;
-struct vhost_dev vhost_dev;
-VhostUserState vhost_user;
-VirtQueue *req_vq;
-bool connected;
-
+VHostUserBase parent;
 /*< public >*/
 };
 
diff --git a/hw/virtio/vhost-user-rng.c b/hw/virtio/vhost-user-rng.c
index efc54cd3fb..01879c863d 100644
--- a/hw/virtio/vhost-user-rng.c
+++ b/hw/virtio/vhost-user-rng.c
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2021 Mathieu Poirier 
  *
- * Implementation seriously tailored on vhost-user-i2c.c
+ * Simple wrapper of the generic vhost-user-device.
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
@@ -13,281 +13,47 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/vhost-user-rng.h"
-#include "qemu/error-report.h"
 #include "standard-headers/linux/virtio_ids.h"
 
-static const int feature_bits[] = {
-VIRTIO_F_RING_RESET,
-VHOST_INVALID_FEATURE_BIT
-};
-
-static void vu_rng_start(VirtIODevice *vdev)
-{
-VHostUserRNG *rng = VHOST_USER_RNG(vdev);
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-int ret;
-int i;
-
-if (!k->set_guest_notifiers) {
-error_report("binding does not support guest notifiers");
-return;
-}
-
-ret = vhost_dev_enable_notifiers(>vhost_dev, vdev);
-if (ret < 0) {
-error_report("Error enabling host notifiers: %d", -ret);
-return;
-}
-
-ret = k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, true);
-if (ret < 0) {
-error_report("Error binding guest notifier: %d", -ret);
-goto err_host_notifiers;
-}
-
-rng->vhost_dev.acked_features = vdev->guest_features;
-ret = vhost_dev_start(>vhost_dev, vdev, true);
-if (ret < 0) {
-error_report("Error starting vhost-user-rng: %d", -ret);
-goto err_guest_notifiers;
-}
-
-/*
- * guest_notifier_mask/pending not used yet, so just unmask
- * everything here. virtio-pci will do the right thing by
- * enabling/disabling irqfd.
- */
-for (i = 0; i < rng->vhost_dev.nvqs; i++) {
-vhost_virtqueue_mask(>vhost_dev, vdev, i, false);
-}
-
-return;
-
-err_guest_notifiers:
-k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, false);
-err_host_notifiers:
-vhost_dev_disable_notifiers(>vhost_dev, vdev);
-}
-
-static void vu_rng_stop(VirtIODevice *vdev)
-{
-VHostUserRNG *rng = VHOST_USER_RNG(vdev);
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-int ret;
-
-if (!k->set_guest_notifiers) {
-return;
-}
-
-vhost_dev_stop(>vhost_dev, vdev, true);
-
-ret = k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, false);
-if (ret < 0) {
-error_report("vhost guest notifier cleanup failed: %d", ret);
-return;
-}
-
-vhost_dev_disable_notifiers(>vhost_dev, vdev);
-}
-
-static void vu_rng_set_status(VirtIODevice *vdev, uint8_t status)
-{
-VHostUserRNG *rng = VHOST_USER_RNG(vdev);
-bool should_start = virtio_device_should_start(vdev, status);
-
-if (vhost_dev_is_started(>vhost_dev) == should_start) {
-return;
-}
-
-if (should_start) {
-vu_rng_start(vdev);
-} else {
-vu_rng_stop(vdev);
-}
-}
-
-static uint64_t vu_rng_get_features(VirtIODevice *vdev,
-uint64_t requested_features, Error **errp)
-{
-VHostUserRNG *rng = VHOST_USER_RNG(vdev);
-
-return vhost_get_features(>vhost_dev, feature_bits,
-  requested_features);
-}
-
-static void vu_rng_handle_output(VirtIODevice *vdev, VirtQueue *vq)
-{
-/*
- * Not normally called; it's the daemon 

[PATCH v4 5/6] hw/virtio: add vhost-user-snd and virtio-snd-pci devices

2023-10-09 Thread Alex Bennée
From: Manos Pitsidianakis 

Tested with rust-vmm vhost-user-sound daemon:

RUST_LOG=trace cargo run --bin vhost-user-sound -- --socket /tmp/snd.sock 
--backend null

Invocation:

qemu-system-x86_64  \
-qmp unix:./qmp-sock,server,wait=off  \
-m 4096 \
-numa node,memdev=mem \
-object 
memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on \
-D qemu.log \
-d guest_errors,trace:\*snd\*,trace:\*sound\*,trace:\*vhost\* \
-chardev socket,id=vsnd,path=/tmp/snd.sock \
-device vhost-user-snd-pci,chardev=vsnd,id=snd \
/path/to/disk

[AJB: imported from 
https://github.com/epilys/qemu-virtio-snd/commit/54ae1cdd15fef2d88e9e387a175f099a38c636f4.patch]
Signed-off-by: Alex Bennée 

---
v1
  - import and test
---
 include/hw/virtio/vhost-user-snd.h | 26 +++
 hw/virtio/vhost-user-snd-pci.c | 75 ++
 hw/virtio/vhost-user-snd.c | 67 ++
 hw/virtio/Kconfig  |  5 ++
 hw/virtio/meson.build  |  3 ++
 5 files changed, 176 insertions(+)
 create mode 100644 include/hw/virtio/vhost-user-snd.h
 create mode 100644 hw/virtio/vhost-user-snd-pci.c
 create mode 100644 hw/virtio/vhost-user-snd.c

diff --git a/include/hw/virtio/vhost-user-snd.h 
b/include/hw/virtio/vhost-user-snd.h
new file mode 100644
index 00..a1627003a0
--- /dev/null
+++ b/include/hw/virtio/vhost-user-snd.h
@@ -0,0 +1,26 @@
+/*
+ * Vhost-user Sound virtio device
+ *
+ * Copyright (c) 2021 Mathieu Poirier 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QEMU_VHOST_USER_SND_H
+#define QEMU_VHOST_USER_SND_H
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user.h"
+#include "hw/virtio/vhost-user-base.h"
+
+#define TYPE_VHOST_USER_SND "vhost-user-snd"
+OBJECT_DECLARE_SIMPLE_TYPE(VHostUserSound, VHOST_USER_SND)
+
+struct VHostUserSound {
+/*< private >*/
+VHostUserBase parent;
+/*< public >*/
+};
+
+#endif /* QEMU_VHOST_USER_SND_H */
diff --git a/hw/virtio/vhost-user-snd-pci.c b/hw/virtio/vhost-user-snd-pci.c
new file mode 100644
index 00..d61cfdae63
--- /dev/null
+++ b/hw/virtio/vhost-user-snd-pci.c
@@ -0,0 +1,75 @@
+/*
+ * Vhost-user Sound virtio device PCI glue
+ *
+ * Copyright (c) 2023 Manos Pitsidianakis 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/vhost-user-snd.h"
+#include "hw/virtio/virtio-pci.h"
+
+struct VHostUserSoundPCI {
+VirtIOPCIProxy parent_obj;
+VHostUserSound vdev;
+};
+
+typedef struct VHostUserSoundPCI VHostUserSoundPCI;
+
+#define TYPE_VHOST_USER_SND_PCI "vhost-user-snd-pci-base"
+
+DECLARE_INSTANCE_CHECKER(VHostUserSoundPCI, VHOST_USER_SND_PCI,
+ TYPE_VHOST_USER_SND_PCI)
+
+static Property vhost_user_snd_pci_properties[] = {
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vhost_user_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+VHostUserSoundPCI *dev = VHOST_USER_SND_PCI(vpci_dev);
+DeviceState *vdev = DEVICE(>vdev);
+
+vpci_dev->nvectors = 1;
+
+qdev_realize(vdev, BUS(_dev->bus), errp);
+}
+
+static void vhost_user_snd_pci_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+k->realize = vhost_user_snd_pci_realize;
+set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+device_class_set_props(dc, vhost_user_snd_pci_properties);
+pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+pcidev_k->device_id = 0; /* Set by virtio-pci based on virtio id */
+pcidev_k->revision = 0x00;
+pcidev_k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
+}
+
+static void vhost_user_snd_pci_instance_init(Object *obj)
+{
+VHostUserSoundPCI *dev = VHOST_USER_SND_PCI(obj);
+
+virtio_instance_init_common(obj, >vdev, sizeof(dev->vdev),
+TYPE_VHOST_USER_SND);
+}
+
+static const VirtioPCIDeviceTypeInfo vhost_user_snd_pci_info = {
+.base_name = TYPE_VHOST_USER_SND_PCI,
+.non_transitional_name = "vhost-user-snd-pci",
+.instance_size = sizeof(VHostUserSoundPCI),
+.instance_init = vhost_user_snd_pci_instance_init,
+.class_init = vhost_user_snd_pci_class_init,
+};
+
+static void vhost_user_snd_pci_register(void)
+{
+virtio_pci_types_register(_user_snd_pci_info);
+}
+
+type_init(vhost_user_snd_pci_register);
diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c
new file mode 100644
index 00..9a217543f8
--- /dev/null
+++ b/hw/virtio/vhost-user-snd.c
@@ -0,0 +1,67 @@
+/*
+ * Vhost-user snd virtio device
+ *
+ * Copyright (c) 2023 Manos Pitsidianakis 
+ *
+ * Simple wrapper of the generic vhost-user-device.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"

[PATCH v4 4/6] hw/virtio: derive vhost-user-i2c from vhost-user-base

2023-10-09 Thread Alex Bennée
Now we can take advantage of the new base class and make
vhost-user-i2c a much simpler boilerplate wrapper. Also as this
doesn't require any target specific hacks we only need to build the
stubs once.

Message-Id: <20230418162140.373219-13-alex.ben...@linaro.org>
Acked-by: Mark Cave-Ayland 
Signed-off-by: Alex Bennée 

---
v2
  - update to new inheritance scheme
  - move build to common code
v3
  - fix merge conflict in meson
  - style updates, remove duplicate includes
v4
  - use vqsize
---
 include/hw/virtio/vhost-user-i2c.h |  14 +-
 hw/virtio/vhost-user-i2c.c | 272 ++---
 hw/virtio/meson.build  |   5 +-
 3 files changed, 23 insertions(+), 268 deletions(-)

diff --git a/include/hw/virtio/vhost-user-i2c.h 
b/include/hw/virtio/vhost-user-i2c.h
index 0f7acd40e3..a8fcb108db 100644
--- a/include/hw/virtio/vhost-user-i2c.h
+++ b/include/hw/virtio/vhost-user-i2c.h
@@ -9,23 +9,17 @@
 #ifndef QEMU_VHOST_USER_I2C_H
 #define QEMU_VHOST_USER_I2C_H
 
+#include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-user.h"
+#include "hw/virtio/vhost-user-base.h"
 
 #define TYPE_VHOST_USER_I2C "vhost-user-i2c-device"
+
 OBJECT_DECLARE_SIMPLE_TYPE(VHostUserI2C, VHOST_USER_I2C)
 
 struct VHostUserI2C {
-VirtIODevice parent;
-CharBackend chardev;
-struct vhost_virtqueue *vhost_vq;
-struct vhost_dev vhost_dev;
-VhostUserState vhost_user;
-VirtQueue *vq;
-bool connected;
+VHostUserBase parent;
 };
 
-/* Virtio Feature bits */
-#define VIRTIO_I2C_F_ZERO_LENGTH_REQUEST   0
-
 #endif /* QEMU_VHOST_USER_I2C_H */
diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
index 4eef3f0633..a464f5e039 100644
--- a/hw/virtio/vhost-user-i2c.c
+++ b/hw/virtio/vhost-user-i2c.c
@@ -14,253 +14,22 @@
 #include "qemu/error-report.h"
 #include "standard-headers/linux/virtio_ids.h"
 
-static const int feature_bits[] = {
-VIRTIO_I2C_F_ZERO_LENGTH_REQUEST,
-VIRTIO_F_RING_RESET,
-VHOST_INVALID_FEATURE_BIT
+static Property vi2c_properties[] = {
+DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+DEFINE_PROP_END_OF_LIST(),
 };
 
-static void vu_i2c_start(VirtIODevice *vdev)
-{
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-int ret, i;
-
-if (!k->set_guest_notifiers) {
-error_report("binding does not support guest notifiers");
-return;
-}
-
-ret = vhost_dev_enable_notifiers(>vhost_dev, vdev);
-if (ret < 0) {
-error_report("Error enabling host notifiers: %d", -ret);
-return;
-}
-
-ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, true);
-if (ret < 0) {
-error_report("Error binding guest notifier: %d", -ret);
-goto err_host_notifiers;
-}
-
-i2c->vhost_dev.acked_features = vdev->guest_features;
-
-ret = vhost_dev_start(>vhost_dev, vdev, true);
-if (ret < 0) {
-error_report("Error starting vhost-user-i2c: %d", -ret);
-goto err_guest_notifiers;
-}
-
-/*
- * guest_notifier_mask/pending not used yet, so just unmask
- * everything here. virtio-pci will do the right thing by
- * enabling/disabling irqfd.
- */
-for (i = 0; i < i2c->vhost_dev.nvqs; i++) {
-vhost_virtqueue_mask(>vhost_dev, vdev, i, false);
-}
-
-return;
-
-err_guest_notifiers:
-k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
-err_host_notifiers:
-vhost_dev_disable_notifiers(>vhost_dev, vdev);
-}
-
-static void vu_i2c_stop(VirtIODevice *vdev)
-{
-VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-int ret;
-
-if (!k->set_guest_notifiers) {
-return;
-}
-
-vhost_dev_stop(>vhost_dev, vdev, true);
-
-ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
-if (ret < 0) {
-error_report("vhost guest notifier cleanup failed: %d", ret);
-return;
-}
-
-vhost_dev_disable_notifiers(>vhost_dev, vdev);
-}
-
-static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status)
-{
-VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-bool should_start = virtio_device_should_start(vdev, status);
-
-if (vhost_dev_is_started(>vhost_dev) == should_start) {
-return;
-}
-
-if (should_start) {
-vu_i2c_start(vdev);
-} else {
-vu_i2c_stop(vdev);
-}
-}
-
-static uint64_t vu_i2c_get_features(VirtIODevice *vdev,
-uint64_t requested_features, Error **errp)
-{
-VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-
-virtio_add_feature(_features, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST);
-return vhost_get_features(>vhost_dev, feature_bits, 
requested_features);
-}
-
-static void vu_i2c_handle_output(VirtIODevice *vdev, 

[PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device

2023-10-09 Thread Alex Bennée
Lets keep a cleaner split between the base class and the derived
vhost-user-device which we can use for generic vhost-user stubs. This
includes an update to introduce the vq_size property so the number of
entries in a virtq can be defined.

Signed-off-by: Alex Bennée 

---
v1
  - merge and re-base, reset testing/review tags
---
 include/hw/virtio/vhost-user-base.h |  49 
 hw/virtio/vhost-user-base.c | 348 
 hw/virtio/vhost-user-device-pci.c   |  10 +-
 hw/virtio/vhost-user-device.c   | 335 +-
 hw/virtio/meson.build   |   1 +
 5 files changed, 410 insertions(+), 333 deletions(-)
 create mode 100644 include/hw/virtio/vhost-user-base.h
 create mode 100644 hw/virtio/vhost-user-base.c

diff --git a/include/hw/virtio/vhost-user-base.h 
b/include/hw/virtio/vhost-user-base.h
new file mode 100644
index 00..cad377468b
--- /dev/null
+++ b/include/hw/virtio/vhost-user-base.h
@@ -0,0 +1,49 @@
+/*
+ * Vhost-user generic virtio device
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QEMU_VHOST_USER_BASE_H
+#define QEMU_VHOST_USER_BASE_H
+
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user.h"
+
+#define TYPE_VHOST_USER_BASE "vhost-user-base"
+
+OBJECT_DECLARE_TYPE(VHostUserBase, VHostUserBaseClass, VHOST_USER_BASE)
+
+struct VHostUserBase {
+VirtIODevice parent;
+
+/* Properties */
+CharBackend chardev;
+uint16_t virtio_id;
+uint32_t num_vqs;
+uint32_t vq_size; /* can't exceed VIRTIO_QUEUE_MAX */
+uint32_t config_size;
+/* State tracking */
+VhostUserState vhost_user;
+struct vhost_virtqueue *vhost_vq;
+struct vhost_dev vhost_dev;
+GPtrArray *vqs;
+bool connected;
+};
+
+/*
+ * Needed so we can use the base realize after specialisation
+ * tweaks
+ */
+struct VHostUserBaseClass {
+VirtioDeviceClass parent_class;
+
+DeviceRealize parent_realize;
+};
+
+
+#define TYPE_VHOST_USER_DEVICE "vhost-user-device"
+
+#endif /* QEMU_VHOST_USER_BASE_H */
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
new file mode 100644
index 00..a8b811c394
--- /dev/null
+++ b/hw/virtio/vhost-user-base.c
@@ -0,0 +1,348 @@
+/*
+ * Base vhost-user-base implementation. This can be used to derive a
+ * more fully specified vhost-user backend either generically (see
+ * vhost-user-device) or via a specific stub for a device which
+ * encapsulates some fixed parameters.
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ * Author: Alex Bennée 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/vhost-user-base.h"
+#include "qemu/error-report.h"
+
+static void vub_start(VirtIODevice *vdev)
+{
+BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+VHostUserBase *vub = VHOST_USER_BASE(vdev);
+int ret, i;
+
+if (!k->set_guest_notifiers) {
+error_report("binding does not support guest notifiers");
+return;
+}
+
+ret = vhost_dev_enable_notifiers(>vhost_dev, vdev);
+if (ret < 0) {
+error_report("Error enabling host notifiers: %d", -ret);
+return;
+}
+
+ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true);
+if (ret < 0) {
+error_report("Error binding guest notifier: %d", -ret);
+goto err_host_notifiers;
+}
+
+vub->vhost_dev.acked_features = vdev->guest_features;
+
+ret = vhost_dev_start(>vhost_dev, vdev, true);
+if (ret < 0) {
+error_report("Error starting vhost-user-base: %d", -ret);
+goto err_guest_notifiers;
+}
+
+/*
+ * guest_notifier_mask/pending not used yet, so just unmask
+ * everything here. virtio-pci will do the right thing by
+ * enabling/disabling irqfd.
+ */
+for (i = 0; i < vub->vhost_dev.nvqs; i++) {
+vhost_virtqueue_mask(>vhost_dev, vdev, i, false);
+}
+
+return;
+
+err_guest_notifiers:
+k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
+err_host_notifiers:
+vhost_dev_disable_notifiers(>vhost_dev, vdev);
+}
+
+static void vub_stop(VirtIODevice *vdev)
+{
+VHostUserBase *vub = VHOST_USER_BASE(vdev);
+BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+int ret;
+
+if (!k->set_guest_notifiers) {
+return;
+}
+
+vhost_dev_stop(>vhost_dev, vdev, true);
+
+ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
+if (ret < 0) {
+error_report("vhost guest notifier cleanup failed: %d", ret);
+return;
+}
+
+vhost_dev_disable_notifiers(>vhost_dev, vdev);
+}
+
+static void vub_set_status(VirtIODevice *vdev, uint8_t status)
+{
+VHostUserBase *vub = VHOST_USER_BASE(vdev);
+bool should_start 

[PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices

2023-10-09 Thread Alex Bennée
Make it clear the vhost-user-device is intended for expert use only.

Signed-off-by: Alex Bennée 

---
v2
  - make clear vhost-user-device for expert use
---
 docs/system/devices/vhost-user-rng.rst |  2 ++
 docs/system/devices/vhost-user.rst | 41 ++
 2 files changed, 43 insertions(+)

diff --git a/docs/system/devices/vhost-user-rng.rst 
b/docs/system/devices/vhost-user-rng.rst
index a145d4105c..ead1405326 100644
--- a/docs/system/devices/vhost-user-rng.rst
+++ b/docs/system/devices/vhost-user-rng.rst
@@ -1,3 +1,5 @@
+.. _vhost_user_rng:
+
 QEMU vhost-user-rng - RNG emulation
 ===
 
diff --git a/docs/system/devices/vhost-user.rst 
b/docs/system/devices/vhost-user.rst
index a80e95a48a..0f9eec3f00 100644
--- a/docs/system/devices/vhost-user.rst
+++ b/docs/system/devices/vhost-user.rst
@@ -15,6 +15,47 @@ to the guest. The code is mostly boilerplate although each 
device has
 a ``chardev`` option which specifies the ID of the ``--chardev``
 device that connects via a socket to the vhost-user *daemon*.
 
+Each device will have an virtio-mmio and virtio-pci variant. See your
+platform details for what sort of virtio bus to use.
+
+.. list-table:: vhost-user devices
+  :widths: 20 20 60
+  :header-rows: 1
+
+  * - Device
+- Type
+- Notes
+  * - vhost-user-device
+- Generic Development Device
+- You must manually specify ``virtio-id`` and the correct ``num_vqs``. 
Intended for expert use.
+  * - vhost-user-blk
+- Block storage
+-
+  * - vhost-user-fs
+- File based storage driver
+- See https://gitlab.com/virtio-fs/virtiofsd
+  * - vhost-user-scsi
+- SCSI based storage
+- See contrib/vhost-user/scsi
+  * - vhost-user-gpio
+- Proxy gpio pins to host
+- See https://github.com/rust-vmm/vhost-device
+  * - vhost-user-i2c
+- Proxy i2c devices to host
+- See https://github.com/rust-vmm/vhost-device
+  * - vhost-user-input
+- Generic input driver
+- See contrib/vhost-user-input
+  * - vhost-user-rng
+- Entropy driver
+- :ref:`vhost_user_rng`
+  * - vhost-user-gpu
+- GPU driver
+-
+  * - vhost-user-vsock
+- Socket based communication
+- See https://github.com/rust-vmm/vhost-device
+
 vhost-user daemon
 =
 
-- 
2.39.2




[PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c

2023-10-09 Thread Alex Bennée
A lot of our vhost-user stubs are large chunks of boilerplate that do
(mostly) the same thing. This series continues the cleanups by
splitting the vhost-user-base and vhost-user-generic implementations.
After adding a new vq_size property the rng, gpio and i2c vhost-user
devices become simple specialisations of the common base defining the
ID, number of queues and potentially the config handling.

I've also added Manos' vhost-user-sound while I was at it.

Changes
---

I've dropped the F_TRANSPORT work from this series to keep this small
and ready to merge. The changes for F_TRANSPORT are a bit more
invasive and still need a bit of debugging but I wanted to get this
stuff merged now.

Alex Bennée (5):
  virtio: split into vhost-user-base and vhost-user-device
  hw/virtio: derive vhost-user-rng from vhost-user-base
  hw/virtio: derive vhost-user-gpio from vhost-user-base
  hw/virtio: derive vhost-user-i2c from vhost-user-base
  docs/system: add a basic enumeration of vhost-user devices

Manos Pitsidianakis (1):
  hw/virtio: add vhost-user-snd and virtio-snd-pci devices

 docs/system/devices/vhost-user-rng.rst |   2 +
 docs/system/devices/vhost-user.rst |  41 +++
 include/hw/virtio/vhost-user-base.h|  49 +++
 include/hw/virtio/vhost-user-gpio.h|  23 +-
 include/hw/virtio/vhost-user-i2c.h |  14 +-
 include/hw/virtio/vhost-user-rng.h |  11 +-
 include/hw/virtio/vhost-user-snd.h |  26 ++
 hw/virtio/vhost-user-base.c| 348 +
 hw/virtio/vhost-user-device-pci.c  |  10 +-
 hw/virtio/vhost-user-device.c  | 335 +---
 hw/virtio/vhost-user-gpio.c| 407 ++---
 hw/virtio/vhost-user-i2c.c | 272 +
 hw/virtio/vhost-user-rng.c | 278 ++---
 hw/virtio/vhost-user-snd-pci.c |  75 +
 hw/virtio/vhost-user-snd.c |  67 
 hw/virtio/Kconfig  |   5 +
 hw/virtio/meson.build  |  25 +-
 17 files changed, 705 insertions(+), 1283 deletions(-)
 create mode 100644 include/hw/virtio/vhost-user-base.h
 create mode 100644 include/hw/virtio/vhost-user-snd.h
 create mode 100644 hw/virtio/vhost-user-base.c
 create mode 100644 hw/virtio/vhost-user-snd-pci.c
 create mode 100644 hw/virtio/vhost-user-snd.c

-- 
2.39.2




Re: [PATCH v7 05/15] python/qemu: rename command() to cmd()

2023-10-09 Thread Juan Quintela
Vladimir Sementsov-Ogievskiy  wrote:
> Use a shorter name. We are going to move in iotests from qmp() to
> command() where possible. But command() is longer than qmp() and don't
> look better. Let's rename.

I feel your pain O:-)

> You can simply grep for '\.command(' and for 'def command(' to check
> that everything is updated (command() in tests/docker/docker.py is
> unrelated).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Daniel P. Berrangé 
> Reviewed-by: Eric Blake 
> [vsementsov: also update three occurrences in
>tests/avocado/machine_aspeed.py and keep r-b]

Reviewed-by: Juan Quintela 




[PATCH v2 08/10] blockjob: query driver-specific info via a new 'query' driver method

2023-10-09 Thread Fiona Ebner
Signed-off-by: Fiona Ebner 
---

No changes in v2.

 blockjob.c   | 4 
 include/block/blockjob_int.h | 5 +
 2 files changed, 9 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index f8cf6e58e2..7e8cfad0fd 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -376,6 +376,7 @@ BlockJobInfo *block_job_query_locked(BlockJob *job, Error 
**errp)
 {
 BlockJobInfo *info;
 uint64_t progress_current, progress_total;
+const BlockJobDriver *drv = block_job_driver(job);
 
 GLOBAL_STATE_CODE();
 
@@ -405,6 +406,9 @@ BlockJobInfo *block_job_query_locked(BlockJob *job, Error 
**errp)
 g_strdup(error_get_pretty(job->job.err)) :
 g_strdup(strerror(-job->job.ret));
 }
+if (drv->query) {
+drv->query(job, info);
+}
 return info;
 }
 
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index f604985315..4ab88b3c97 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -72,6 +72,11 @@ struct BlockJobDriver {
  * Change the @job's options according to @opts.
  */
 void (*change)(BlockJob *job, BlockJobChangeOptions *opts, Error **errp);
+
+/*
+ * Query information specific to this kind of block job.
+ */
+void (*query)(BlockJob *job, BlockJobInfo *info);
 };
 
 /*
-- 
2.39.2





[PATCH v2 09/10] mirror: return mirror-specific information upon query

2023-10-09 Thread Fiona Ebner
To start out, only actively-synced is returned.

For example, this is useful for jobs that started out in background
mode and switched to active mode. Once actively-synced is true, it's
clear that the mode switch has been completed. Note that completion of
the switch might happen much earlier, e.g. if the switch happens
before the job is ready, once all background operations have finished.
It's assumed that whether the disks are actively-synced or not is more
interesting than whether the mode switch completed. That information
can still be added if required in the future.

Signed-off-by: Fiona Ebner 
---

Changes in v2:
* udpate QEMU version in QAPI
* update indentation in QAPI (like in a937b6aa73 ("qapi: Reformat
  doc comments to conform to current conventions"))

 block/mirror.c   | 10 ++
 qapi/block-core.json | 16 +++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 83aa4176c2..33b72ec5e5 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1267,6 +1267,15 @@ static void mirror_change(BlockJob *job, 
BlockJobChangeOptions *opts,
 s->copy_mode = MIRROR_COPY_MODE_WRITE_BLOCKING;
 }
 
+static void mirror_query(BlockJob *job, BlockJobInfo *info)
+{
+MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+info->u.mirror = (BlockJobInfoMirror) {
+.actively_synced = s->actively_synced,
+};
+}
+
 static const BlockJobDriver mirror_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(MirrorBlockJob),
@@ -1282,6 +1291,7 @@ static const BlockJobDriver mirror_job_driver = {
 },
 .drained_poll   = mirror_drained_poll,
 .change = mirror_change,
+.query  = mirror_query,
 };
 
 static const BlockJobDriver commit_active_job_driver = {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 950542b735..35d67410cc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1352,6 +1352,20 @@
 { 'enum': 'MirrorCopyMode',
   'data': ['background', 'write-blocking'] }
 
+##
+# @BlockJobInfoMirror:
+#
+# Information specific to mirror block jobs.
+#
+# @actively-synced: Whether the source is actively synced to the
+# target, i.e. same data and new writes are done synchronously to
+# both.
+#
+# Since 8.2
+##
+{ 'struct': 'BlockJobInfoMirror',
+  'data': { 'actively-synced': 'bool' } }
+
 ##
 # @BlockJobInfo:
 #
@@ -1403,7 +1417,7 @@
'auto-finalize': 'bool', 'auto-dismiss': 'bool',
'*error': 'str' },
   'discriminator': 'type',
-  'data': {} }
+  'data': { 'mirror': 'BlockJobInfoMirror' } }
 
 ##
 # @query-block-jobs:
-- 
2.39.2





[PATCH v2 04/10] block/mirror: determine copy_to_target only once

2023-10-09 Thread Fiona Ebner
In preparation to allow changing the copy_mode via QMP. When running
in an iothread, it could be that copy_mode is changed from the main
thread in between reading copy_mode in bdrv_mirror_top_pwritev() and
reading copy_mode in bdrv_mirror_top_do_write(), so they might end up
disagreeing about whether copy_to_target is true or false. Avoid that
scenario by determining copy_to_target only once and passing it to
bdrv_mirror_top_do_write() as an argument.

Signed-off-by: Fiona Ebner 
---

New in v2.

 block/mirror.c | 41 ++---
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 0ed54754e2..8992c09172 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1463,21 +1463,21 @@ bdrv_mirror_top_preadv(BlockDriverState *bs, int64_t 
offset, int64_t bytes,
 return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
 
+static bool should_copy_to_target(MirrorBDSOpaque *s)
+{
+return s->job && s->job->ret >= 0 &&
+!job_is_cancelled(>job->common.job) &&
+s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING;
+}
+
 static int coroutine_fn GRAPH_RDLOCK
 bdrv_mirror_top_do_write(BlockDriverState *bs, MirrorMethod method,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+ bool copy_to_target, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
 {
 MirrorOp *op = NULL;
 MirrorBDSOpaque *s = bs->opaque;
 int ret = 0;
-bool copy_to_target = false;
-
-if (s->job) {
-copy_to_target = s->job->ret >= 0 &&
- !job_is_cancelled(>job->common.job) &&
- s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING;
-}
 
 if (copy_to_target) {
 op = active_write_prepare(s->job, offset, bytes);
@@ -1523,17 +1523,10 @@ static int coroutine_fn GRAPH_RDLOCK
 bdrv_mirror_top_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
 QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
-MirrorBDSOpaque *s = bs->opaque;
 QEMUIOVector bounce_qiov;
 void *bounce_buf;
 int ret = 0;
-bool copy_to_target = false;
-
-if (s->job) {
-copy_to_target = s->job->ret >= 0 &&
- !job_is_cancelled(>job->common.job) &&
- s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING;
-}
+bool copy_to_target = should_copy_to_target(bs->opaque);
 
 if (copy_to_target) {
 /* The guest might concurrently modify the data to write; but
@@ -1550,8 +1543,8 @@ bdrv_mirror_top_pwritev(BlockDriverState *bs, int64_t 
offset, int64_t bytes,
 flags &= ~BDRV_REQ_REGISTERED_BUF;
 }
 
-ret = bdrv_mirror_top_do_write(bs, MIRROR_METHOD_COPY, offset, bytes, qiov,
-   flags);
+ret = bdrv_mirror_top_do_write(bs, MIRROR_METHOD_COPY, copy_to_target,
+   offset, bytes, qiov, flags);
 
 if (copy_to_target) {
 qemu_iovec_destroy(_qiov);
@@ -1574,15 +1567,17 @@ static int coroutine_fn GRAPH_RDLOCK
 bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
   int64_t bytes, BdrvRequestFlags flags)
 {
-return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_ZERO, offset, bytes, 
NULL,
-flags);
+bool copy_to_target = should_copy_to_target(bs->opaque);
+return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_ZERO, copy_to_target,
+offset, bytes, NULL, flags);
 }
 
 static int coroutine_fn GRAPH_RDLOCK
 bdrv_mirror_top_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
-return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_DISCARD, offset, bytes,
-NULL, 0);
+bool copy_to_target = should_copy_to_target(bs->opaque);
+return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_DISCARD, copy_to_target,
+offset, bytes, NULL, 0);
 }
 
 static void bdrv_mirror_top_refresh_filename(BlockDriverState *bs)
-- 
2.39.2





[PATCH v2 07/10] qapi/block-core: turn BlockJobInfo into a union

2023-10-09 Thread Fiona Ebner
In preparation to additionally return job-type-specific information.

Signed-off-by: Fiona Ebner 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---

No changes in v2.

 qapi/block-core.json | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index a19718a69f..950542b735 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1395,13 +1395,15 @@
 #
 # Since: 1.1
 ##
-{ 'struct': 'BlockJobInfo',
-  'data': {'type': 'JobType', 'device': 'str', 'len': 'int',
+{ 'union': 'BlockJobInfo',
+  'base': {'type': 'JobType', 'device': 'str', 'len': 'int',
'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
'io-status': 'BlockDeviceIoStatus', 'ready': 'bool',
'status': 'JobStatus',
'auto-finalize': 'bool', 'auto-dismiss': 'bool',
-   '*error': 'str' } }
+   '*error': 'str' },
+  'discriminator': 'type',
+  'data': {} }
 
 ##
 # @query-block-jobs:
-- 
2.39.2





[PATCH v2 05/10] mirror: implement mirror_change method

2023-10-09 Thread Fiona Ebner
which allows switching the @copy-mode from 'background' to
'write-blocking'.

This is useful for management applications, so they can start out in
background mode to avoid limiting guest write speed and switch to
active mode when certain criteria are fulfilled.

Signed-off-by: Fiona Ebner 
---

Changes in v2:
* update QEMU version in QAPI
* update indentation in QAPI (like in a937b6aa73 ("qapi: Reformat
  doc comments to conform to current conventions"))
* drop drained section and disable dirty bitmap call. It's already
  disabled, because the bitmap is now attached to the filter and
  set in bdrv_mirror_top_do_write(). See the earlier patch
  "block/mirror: move dirty bitmap to filter"

 block/mirror.c   | 22 ++
 qapi/block-core.json | 13 -
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index b84de56734..83aa4176c2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1246,6 +1246,27 @@ static bool commit_active_cancel(Job *job, bool force)
 return force || !job_is_ready(job);
 }
 
+static void mirror_change(BlockJob *job, BlockJobChangeOptions *opts,
+  Error **errp)
+{
+MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+BlockJobChangeOptionsMirror *change_opts = >u.mirror;
+
+if (s->copy_mode == change_opts->copy_mode) {
+return;
+}
+
+if (s->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING) {
+error_setg(errp, "Cannot switch away from copy mode 'write-blocking'");
+return;
+}
+
+assert(s->copy_mode == MIRROR_COPY_MODE_BACKGROUND &&
+   change_opts->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING);
+
+s->copy_mode = MIRROR_COPY_MODE_WRITE_BLOCKING;
+}
+
 static const BlockJobDriver mirror_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(MirrorBlockJob),
@@ -1260,6 +1281,7 @@ static const BlockJobDriver mirror_job_driver = {
 .cancel = mirror_cancel,
 },
 .drained_poll   = mirror_drained_poll,
+.change = mirror_change,
 };
 
 static const BlockJobDriver commit_active_job_driver = {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c6f31a9399..01427c259a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3044,6 +3044,17 @@
 { 'command': 'block-job-finalize', 'data': { 'id': 'str' },
   'allow-preconfig': true }
 
+##
+# @BlockJobChangeOptionsMirror:
+#
+# @copy-mode: Switch to this copy mode. Currenlty, only the switch
+# from 'background' to 'write-blocking' is implemented.
+#
+# Since: 8.2
+##
+{ 'struct': 'BlockJobChangeOptionsMirror',
+  'data': { 'copy-mode' : 'MirrorCopyMode' } }
+
 ##
 # @BlockJobChangeOptions:
 #
@@ -3058,7 +3069,7 @@
 { 'union': 'BlockJobChangeOptions',
   'base': { 'id': 'str', 'type': 'JobType' },
   'discriminator': 'type',
-  'data': {} }
+  'data': { 'mirror': 'BlockJobChangeOptionsMirror' } }
 
 ##
 # @block-job-change:
-- 
2.39.2





[PATCH v2 10/10] iotests: adapt test output for new mirror query property

2023-10-09 Thread Fiona Ebner
Signed-off-by: Fiona Ebner 
---

New in v2.

 tests/qemu-iotests/109.out | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
index 2611d6a40f..965c9a6a0a 100644
--- a/tests/qemu-iotests/109.out
+++ b/tests/qemu-iotests/109.out
@@ -38,7 +38,7 @@ read 512/512 bytes at offset 0
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, 
"speed": 0, "type": "mirror"}}
 {"execute":"query-block-jobs"}
-{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 1024, "offset": 1024, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 1024, "offset": 1024, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror", 
"actively-synced": false}]}
 {"execute":"quit"}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
@@ -90,7 +90,7 @@ read 512/512 bytes at offset 0
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "src", "len": 197120, "offset": 197120, 
"speed": 0, "type": "mirror"}}
 {"execute":"query-block-jobs"}
-{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 197120, "offset": 197120, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 197120, "offset": 197120, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror", 
"actively-synced": false}]}
 {"execute":"quit"}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
@@ -142,7 +142,7 @@ read 512/512 bytes at offset 0
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "src", "len": 327680, "offset": 327680, 
"speed": 0, "type": "mirror"}}
 {"execute":"query-block-jobs"}
-{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 327680, "offset": 327680, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 327680, "offset": 327680, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror", 
"actively-synced": false}]}
 {"execute":"quit"}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
@@ -194,7 +194,7 @@ read 512/512 bytes at offset 0
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, 
"speed": 0, "type": "mirror"}}
 {"execute":"query-block-jobs"}
-{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 1024, "offset": 1024, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"return": [{"auto-finalize": true, "io-status": "ok", "device": "src", 
"auto-dismiss": true, "busy": false, "len": 1024, "offset": 1024, "status": 
"ready", "paused": false, "speed": 0, "ready": true, "type": "mirror", 
"actively-synced": false}]}
 {"execute":"quit"}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
@@ -246,7 +246,7 @@ read 512/512 bytes at offset 0
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, 
"speed": 0, "type": "mirror"}}
 

[PATCH v2 02/10] block/mirror: set actively_synced even after the job is ready

2023-10-09 Thread Fiona Ebner
In preparation to allow switching from background to active mode. This
ensures that setting actively_synced will not be missed when the
switch happens after the job is ready.

Signed-off-by: Fiona Ebner 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---

No changes in v2.

 block/mirror.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 3cc0757a03..b764ad5108 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1074,9 +1074,9 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
  * the target in a consistent state.
  */
 job_transition_to_ready(>common.job);
-if (s->copy_mode != MIRROR_COPY_MODE_BACKGROUND) {
-s->actively_synced = true;
-}
+}
+if (s->copy_mode != MIRROR_COPY_MODE_BACKGROUND) {
+s->actively_synced = true;
 }
 
 should_complete = s->should_complete ||
-- 
2.39.2





[PATCH v2 00/10] mirror: allow switching from background to active mode

2023-10-09 Thread Fiona Ebner
Changes in v2:
* move bitmap to filter which allows to avoid draining when
  changing the copy mode
* add patch to determine copy_to_target only once
* drop patches returning redundant information upon query
* update QEMU version in QAPI
* update indentation in QAPI
* update indentation in QAPI (like in a937b6aa73 ("qapi: Reformat
  doc comments to conform to current conventions"))
* add patch to adapt iotest output

Discussion of v1:
https://lists.nongnu.org/archive/html/qemu-devel/2023-02/msg07216.html

With active mode, the guest write speed is limited by the synchronous
writes to the mirror target. For this reason, management applications
might want to start out in background mode and only switch to active
mode later, when certain conditions are met. This series adds a
block-job-change QMP command to achieve that, as well as
job-type-specific information when querying block jobs, which
can be used to decide when the switch should happen.

For now, only the direction background -> active is supported.

The information added upon querying is whether the target is actively
synced, the total data sent, and the remaining dirty bytes.

Initially, I tried to go for a more general 'job-change' command, but
I couldn't figure out a way to avoid mutual inclusion between
block-core.json and job.json.


Fiona Ebner (10):
  blockjob: introduce block-job-change QMP command
  block/mirror: set actively_synced even after the job is ready
  block/mirror: move dirty bitmap to filter
  block/mirror: determine copy_to_target only once
  mirror: implement mirror_change method
  qapi/block-core: use JobType for BlockJobInfo's type
  qapi/block-core: turn BlockJobInfo into a union
  blockjob: query driver-specific info via a new 'query' driver method
  mirror: return mirror-specific information upon query
  iotests: adapt test output for new mirror query property

 block/mirror.c | 95 +++---
 block/monitor/block-hmp-cmds.c |  4 +-
 blockdev.c | 14 +
 blockjob.c | 26 +-
 include/block/blockjob.h   | 11 
 include/block/blockjob_int.h   | 10 
 job.c  |  1 +
 qapi/block-core.json   | 59 +++--
 qapi/job.json  |  4 +-
 tests/qemu-iotests/109.out | 24 -
 10 files changed, 199 insertions(+), 49 deletions(-)

-- 
2.39.2





[PATCH v2 06/10] qapi/block-core: use JobType for BlockJobInfo's type

2023-10-09 Thread Fiona Ebner
In preparation to turn BlockJobInfo into a union with @type as the
discriminator. That requires it to be an enum.

No functional change is intended.

Signed-off-by: Fiona Ebner 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---

No changes in v2.

 block/monitor/block-hmp-cmds.c | 4 ++--
 blockjob.c | 2 +-
 qapi/block-core.json   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index ca2599de44..f9f87e5c47 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -843,7 +843,7 @@ void hmp_info_block_jobs(Monitor *mon, const QDict *qdict)
 }
 
 while (list) {
-if (strcmp(list->value->type, "stream") == 0) {
+if (list->value->type == JOB_TYPE_STREAM) {
 monitor_printf(mon, "Streaming device %s: Completed %" PRId64
" of %" PRId64 " bytes, speed limit %" PRId64
" bytes/s\n",
@@ -855,7 +855,7 @@ void hmp_info_block_jobs(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, "Type %s, device %s: Completed %" PRId64
" of %" PRId64 " bytes, speed limit %" PRId64
" bytes/s\n",
-   list->value->type,
+   JobType_str(list->value->type),
list->value->device,
list->value->offset,
list->value->len,
diff --git a/blockjob.c b/blockjob.c
index d53bc775d2..f8cf6e58e2 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -388,7 +388,7 @@ BlockJobInfo *block_job_query_locked(BlockJob *job, Error 
**errp)
   _total);
 
 info = g_new0(BlockJobInfo, 1);
-info->type  = g_strdup(job_type_str(>job));
+info->type  = job_type(>job);
 info->device= g_strdup(job->job.id);
 info->busy  = job->job.busy;
 info->paused= job->job.pause_count > 0;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 01427c259a..a19718a69f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1396,7 +1396,7 @@
 # Since: 1.1
 ##
 { 'struct': 'BlockJobInfo',
-  'data': {'type': 'str', 'device': 'str', 'len': 'int',
+  'data': {'type': 'JobType', 'device': 'str', 'len': 'int',
'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
'io-status': 'BlockDeviceIoStatus', 'ready': 'bool',
'status': 'JobStatus',
-- 
2.39.2





[PATCH v2 01/10] blockjob: introduce block-job-change QMP command

2023-10-09 Thread Fiona Ebner
which will allow changing job-type-specific options after job
creation.

In the JobVerbTable, the same allow bits as for set-speed are used,
because set-speed can be considered an existing change command.

Signed-off-by: Fiona Ebner 
---

Changes in v2:
* update QEMU version in QAPI
* fix typo in function comment

 blockdev.c   | 14 ++
 blockjob.c   | 20 
 include/block/blockjob.h | 11 +++
 include/block/blockjob_int.h |  5 +
 job.c|  1 +
 qapi/block-core.json | 26 ++
 qapi/job.json|  4 +++-
 7 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 325b7a3bef..d0e274ff8b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3344,6 +3344,20 @@ void qmp_block_job_dismiss(const char *id, Error **errp)
 job_dismiss_locked(, errp);
 }
 
+void qmp_block_job_change(BlockJobChangeOptions *opts, Error **errp)
+{
+BlockJob *job;
+
+JOB_LOCK_GUARD();
+job = find_block_job_locked(opts->id, errp);
+
+if (!job) {
+return;
+}
+
+block_job_change_locked(job, opts, errp);
+}
+
 void qmp_change_backing_file(const char *device,
  const char *image_node_name,
  const char *backing_file,
diff --git a/blockjob.c b/blockjob.c
index 58c5d64539..d53bc775d2 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -328,6 +328,26 @@ static bool block_job_set_speed(BlockJob *job, int64_t 
speed, Error **errp)
 return block_job_set_speed_locked(job, speed, errp);
 }
 
+void block_job_change_locked(BlockJob *job, BlockJobChangeOptions *opts,
+ Error **errp)
+{
+const BlockJobDriver *drv = block_job_driver(job);
+
+GLOBAL_STATE_CODE();
+
+if (job_apply_verb_locked(>job, JOB_VERB_CHANGE, errp)) {
+return;
+}
+
+if (drv->change) {
+job_unlock();
+drv->change(job, opts, errp);
+job_lock();
+} else {
+error_setg(errp, "Job type does not support change");
+}
+}
+
 void block_job_ratelimit_processed_bytes(BlockJob *job, uint64_t n)
 {
 IO_CODE();
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 058b0c824c..95854f1477 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -172,6 +172,17 @@ bool block_job_has_bdrv(BlockJob *job, BlockDriverState 
*bs);
  */
 bool block_job_set_speed_locked(BlockJob *job, int64_t speed, Error **errp);
 
+/**
+ * block_job_change_locked:
+ * @job: The job to change.
+ * @opts: The new options.
+ * @errp: Error object.
+ *
+ * Change the job according to opts.
+ */
+void block_job_change_locked(BlockJob *job, BlockJobChangeOptions *opts,
+ Error **errp);
+
 /**
  * block_job_query_locked:
  * @job: The job to get information about.
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 104824040c..f604985315 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -67,6 +67,11 @@ struct BlockJobDriver {
 void (*attached_aio_context)(BlockJob *job, AioContext *new_context);
 
 void (*set_speed)(BlockJob *job, int64_t speed);
+
+/*
+ * Change the @job's options according to @opts.
+ */
+void (*change)(BlockJob *job, BlockJobChangeOptions *opts, Error **errp);
 };
 
 /*
diff --git a/job.c b/job.c
index 72d57f0934..99a2e54b54 100644
--- a/job.c
+++ b/job.c
@@ -80,6 +80,7 @@ bool JobVerbTable[JOB_VERB__MAX][JOB_STATUS__MAX] = {
 [JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0},
 [JOB_VERB_FINALIZE] = {0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0},
 [JOB_VERB_DISMISS]  = {0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0},
+[JOB_VERB_CHANGE]   = {0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0},
 };
 
 /* Transactional group of jobs */
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 89751d81f2..c6f31a9399 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3044,6 +3044,32 @@
 { 'command': 'block-job-finalize', 'data': { 'id': 'str' },
   'allow-preconfig': true }
 
+##
+# @BlockJobChangeOptions:
+#
+# Block job options that can be changed after job creation.
+#
+# @id: The job identifier
+#
+# @type: The job type
+#
+# Since 8.2
+##
+{ 'union': 'BlockJobChangeOptions',
+  'base': { 'id': 'str', 'type': 'JobType' },
+  'discriminator': 'type',
+  'data': {} }
+
+##
+# @block-job-change:
+#
+# Change the block job's options.
+#
+# Since: 8.2
+##
+{ 'command': 'block-job-change',
+  'data': 'BlockJobChangeOptions', 'boxed': true }
+
 ##
 # @BlockdevDiscardOptions:
 #
diff --git a/qapi/job.json b/qapi/job.json
index 7f0ba090de..b3957207a4 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -105,11 +105,13 @@
 #
 # @finalize: see @job-finalize
 #
+# @change: see @block-job-change (since 8.2)
+#
 # Since: 2.12
 ##
 { 'enum': 'JobVerb',
   'data': ['cancel', 'pause', 'resume', 

[PATCH v2 03/10] block/mirror: move dirty bitmap to filter

2023-10-09 Thread Fiona Ebner
In preparation to allow switching to active mode without draining.
Initialization of the bitmap in mirror_dirty_init() still happens with
the original/backing BlockDriverState, which should be fine, because
the mirror top has the same length.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Fiona Ebner 
---

New in v2.

 block/mirror.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index b764ad5108..0ed54754e2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1500,6 +1500,10 @@ bdrv_mirror_top_do_write(BlockDriverState *bs, 
MirrorMethod method,
 abort();
 }
 
+if (!copy_to_target && s->job && s->job->dirty_bitmap) {
+bdrv_set_dirty_bitmap(s->job->dirty_bitmap, offset, bytes);
+}
+
 if (ret < 0) {
 goto out;
 }
@@ -1823,13 +1827,17 @@ static BlockJob *mirror_start_job(
 s->should_complete = true;
 }
 
-s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
+s->dirty_bitmap = bdrv_create_dirty_bitmap(s->mirror_top_bs, granularity,
+   NULL, errp);
 if (!s->dirty_bitmap) {
 goto fail;
 }
-if (s->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING) {
-bdrv_disable_dirty_bitmap(s->dirty_bitmap);
-}
+
+/*
+ * The dirty bitmap is set by bdrv_mirror_top_do_write() when not in active
+ * mode.
+ */
+bdrv_disable_dirty_bitmap(s->dirty_bitmap);
 
 ret = block_job_add_bdrv(>common, "source", bs, 0,
  BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE |
-- 
2.39.2





Re: [PATCH v2 03/21] preallocate: Don't poll during permission updates

2023-10-09 Thread Denis V. Lunev

On 10/6/23 20:10, Vladimir Sementsov-Ogievskiy wrote:

On 06.10.23 11:56, Kevin Wolf wrote:

Am 05.10.2023 um 21:55 hat Vladimir Sementsov-Ogievskiy geschrieben:

On 11.09.23 12:46, Kevin Wolf wrote:
When the permission related BlockDriver callbacks are called, we 
are in
the middle of an operation traversing the block graph. Polling in 
such a

place is a very bad idea because the graph could change in unexpected
ways. In the future, callers will also hold the graph lock, which is
likely to turn polling into a deadlock.

So we need to get rid of calls to functions like bdrv_getlength() or
bdrv_truncate() there as these functions poll internally. They are
currently used so that when no parent has write/resize permissions on
the image any more, the preallocate filter drops the extra 
preallocated

area in the image file and gives up write/resize permissions itself.

In order to achieve this without polling in .bdrv_check_perm, don't
immediately truncate the image, but only schedule a BH to do so. The
filter keeps the write/resize permissions a bit longer now until 
the BH

has executed.

There is one case in which delaying doesn't work: Reopening the image
read-only. In this case, bs->file will likely be reopened read-only,
too, so keeping write permissions a bit longer on it doesn't work. But
we can already cover this case in preallocate_reopen_prepare() and not
rely on the permission updates for it.


Hmm, now I found one more "future" case.

I now try to rebase my "[PATCH v7 0/7] blockdev-replace"
https://patchew.org/QEMU/20230421114102.884457-1-vsement...@yandex-team.ru/ 



And it breaks after this commit.

By accident, blockdev-replace series uses exactly "preallocate" filter
to test insertion/removing of filters. And removing is broken now.

Removing is done as follows:

1. We have filter inserted: disk0 -file-> filter -file-> file0

2. blockdev-replace, replaces file child of disk0, so we should get 
the picture*: disk0 -file-> file0 <-file- filter


3. blockdev-del filter


But step [2] fails, as now preallocate filter doesn't drop permissions
during the operation (postponing this for a while) and the picture* is
impossible. Permission check fails.

Hmmm... Any idea how blockdev-replace and preallocate filter should
work :) ? Maybe, doing truncation in .drain_begin() will help? Will
try


Hm... What preallocate tries to do is really tricky...

Of course, the error is correct, this is an invalid configuration if
preallocate can still resize the image. So it would have to truncate the
file earlier, but the first time that preallocate knows of the change is
already too late to run requests.

Truncating on drain_begin feels more like a hack, but as long as it does
the job... Of course, this will have the preallocation truncated away on
events that have nothing to do with removing the filter. It's not
necessarily a disaster because preallocation is only an optimisation,
but it doesn't feel great.


Hmm, yes, that's not good.



Maybe let's take a step back: Which scenario is the preallocate driver
meant for and why do we even need to truncate the image file after
removing the filter? I suppose the filter doesn't make sense with raw
images because these are fixed size anyway, and pretty much any other
image format should be able to tolerate a permanently rounded up file
size. As long as you don't write to the preallocated area, it shouldn't
take space either on any sane filesystem.

Hmm, actually both VHD and VMDK can have footers, better avoid it with
those... But if truncating the image file on close is critical, what do
you do on crashes? Maybe preallocate should just not be considered
compatible with these formats?



Originally preallocate filter was made to be used with qcow2, on some 
proprietary storage, where:


1. Allocating of big chunk works a lot fater than allocating several 
smaller chunks
2. Holes are not free and/or file length is not free, so we really 
want to truncate the file back on close


Den, correct me if I'm wrong.


1. Absolutely correct. This is true when the file attributes
    are stored in a centralized place aka metadata storage
    and requests to it does not scale well.

2. This is at my opinion has different meaning. We have
    tried to make local storage behavior and distributed
    storage behavior to be the same when VM is off, i.e.
    the file should be in the same state (no free blocks
    at the end of the file).



Good thing is that in this scenario we don't need to remove the filter 
in runtime, so there is no problem.



Yes, this filter is not dynamic in that respect. It is either
here or not here.




Now I think that the generic solution is just add a new handler 
.bdrv_pre_replace, so blockdev-replace may work as follows:


drain_begin

call .bdrv_pre_replace for all affected nodes

do the replace

drain_end

And prellocate filter would do truncation in this .bdrv_pre_replace 
handler and set some flag, that we have nothing to trunctate (the flag 
is 

Re: [PATCH v4 2/4] qcow2: add configurations for zoned format extension

2023-10-09 Thread Sam Li
Hello Eric,

Eric Blake  于2023年9月28日周四 23:15写道:
>
> On Mon, Sep 18, 2023 at 05:53:11PM +0800, Sam Li wrote:
> > To configure the zoned format feature on the qcow2 driver, it
> > requires settings as: the device size, zone model, zone size,
> > zone capacity, number of conventional zones, limits on zone
> > resources (max append sectors, max open zones, and max_active_zones).
> >
> > To create a qcow2 file with zoned format, use command like this:
> > $ qemu-img create -f qcow2 test.qcow2 -o size=768M -o
> > zone_size=64M -o zone_capacity=64M -o nr_conv_zones=0 -o
> > max_append_sectors=512 -o max_open_zones=0 -o max_active_zones=0
> > -o zone_model=1
> >
> > Signed-off-by: Sam Li 
> > ---
> >  block/qcow2.c| 186 ++-
> >  block/qcow2.h|  28 +
> >  docs/interop/qcow2.txt   |  36 ++
> >  include/block/block_int-common.h |  13 +++
> >  qapi/block-core.json |  30 -
> >  5 files changed, 291 insertions(+), 2 deletions(-)
>
> Below, I'll focus only on the spec change, not the implementation:
>
> >
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index b48cd9ce63..521276fc51 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -73,6 +73,7 @@ typedef struct {
> >  #define  QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77
> >  #define  QCOW2_EXT_MAGIC_BITMAPS 0x23852875
> >  #define  QCOW2_EXT_MAGIC_DATA_FILE 0x44415441
> > +#define  QCOW2_EXT_MAGIC_ZONED_FORMAT 0x7a6264
>
> Why not spell it 0x007a6264 with 8 hex digits, like the others?  (I
> get why you choose that constant, though - ascii 'zbd')
>
> > +++ b/docs/interop/qcow2.txt
> > @@ -331,6 +331,42 @@ The fields of the bitmaps extension are:
> > Offset into the image file at which the bitmap directory
> > starts. Must be aligned to a cluster boundary.
> >
> > +== Zoned extension ==
>
> Where is the magic number for this extension called out?  That's
> missing, and MUST be part of the spec.

It's a part of the header extension type in the spec. I will add it.

>
> Back-compatibility constraints: you should consider what happens in
> both of the following cases:
>
> a program that intends to do read-only access to the qcow2 file but
> which does not understand this extension header (for example, an older
> version of 'qemu-img convert' being used to extract data from a newer
> .qcow2 file with this header present - but also the new 'nbdkit
> qcow2dec' decoder plugin just released in nbdkit 1.36).  Is it safe to
> read the data as-is, by basically ignoring zone informations?  Or will
> that ever produce wrong data (for example, if operations on a
> particular zone imply that the guest should read all zeroes after the
> current zone offset within that zone, regardless of whether non-zero
> content was previously stored at those offsets - then not honoring the
> existence of the extension header would require you to add and
> document an incompatible feature bit so that reader apps fail to open
> the file rather than reading wrong data).
>
> a program that intends to edit the qcow2 file but which does not
> understand this extension header (again, consider access by an older
> version of qemu).  Is it safe to just write data anywhere in the disk,
> but where failure to update the zone metadata means that all
> subsequent use of the file MUST behave as if it is now a non-zeoned
> device?  If so, then it is sufficient to document an autoclear feature
> bit: any time a newer qcow2 writer creates a file with a zoned
> extension, it also sets the autoclear feature bit; any time an older
> qcow2 writer edits a file with the autoclear bit, it clears the bit
> (because it has no idea if its edits invalidated the unknown
> extension).  Then when the new qcow2 program again accesses the file,
> it knows that the zone information is no longer reliable, and can fall
> back to forcing the image to behave as flat.

Considering access by an older version of qemu ('old qemu' for abbr.)
with a qcow2 file created with zoned extension ('new file' for abbr.),
reads from a new file on old qemu which does not understand zoned
information are safe. The zoned extension represents necessary zone
states for all zones, which puts constraints to operations on the
zones. For example, writes to offsets that are over the capacity of
that zone are not allowed, where it will be read as zeroes. The old
qemu ignores that and reads the new file as a regular one anyway.

However, what is unsafe is when an old qemu program gets involved in
editing a new file. The new qemu will not see the write pointer
changes of the new file that was done sometime by old qemu programs.
Then the zone information is no longer reliable as you illustrated.

Therefore I will add an autoclear bit for the latter case. It clears
the zoned extension when it is set by old qemu programs.

>
> > +
> > +The zoned extension is an optional header extension. It contains fields for
> > 

Re: [PATCH v7 05/15] python/qemu: rename command() to cmd()

2023-10-09 Thread Cédric Le Goater

On 10/6/23 17:41, Vladimir Sementsov-Ogievskiy wrote:

Use a shorter name. We are going to move in iotests from qmp() to
command() where possible. But command() is longer than qmp() and don't
look better. Let's rename.

You can simply grep for '\.command(' and for 'def command(' to check
that everything is updated (command() in tests/docker/docker.py is
unrelated).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Eric Blake 
[vsementsov: also update three occurrences in
tests/avocado/machine_aspeed.py and keep r-b]


For aspeed,

Reviewed-by: Cédric Le Goater 

Thanks,

C.





---
  docs/devel/testing.rst|  10 +-
  python/qemu/machine/machine.py|   8 +-
  python/qemu/qmp/legacy.py |   2 +-
  python/qemu/qmp/qmp_shell.py  |   2 +-
  python/qemu/utils/qemu_ga_client.py   |   2 +-
  python/qemu/utils/qom.py  |   8 +-
  python/qemu/utils/qom_common.py   |   2 +-
  python/qemu/utils/qom_fuse.py |   6 +-
  scripts/cpu-x86-uarch-abi.py  |   8 +-
  scripts/device-crash-test |   8 +-
  scripts/render_block_graph.py |   8 +-
  tests/avocado/avocado_qemu/__init__.py|   4 +-
  tests/avocado/cpu_queries.py  |   5 +-
  tests/avocado/hotplug_cpu.py  |  10 +-
  tests/avocado/info_usernet.py |   4 +-
  tests/avocado/machine_arm_integratorcp.py |   6 +-
  tests/avocado/machine_aspeed.py   |  12 +-
  tests/avocado/machine_m68k_nextcube.py|   4 +-
  tests/avocado/machine_mips_malta.py   |   6 +-
  tests/avocado/machine_s390_ccw_virtio.py  |  28 ++--
  tests/avocado/migration.py|  10 +-
  tests/avocado/pc_cpu_hotplug_props.py |   2 +-
  tests/avocado/version.py  |   4 +-
  tests/avocado/virtio_check_params.py  |   6 +-
  tests/avocado/virtio_version.py   |   5 +-
  tests/avocado/x86_cpu_model_versions.py   |  13 +-
  tests/migration/guestperf/engine.py   | 150 +++---
  tests/qemu-iotests/256|  34 ++---
  tests/qemu-iotests/257|  36 +++---
  29 files changed, 204 insertions(+), 199 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 5d1fc0aa95..21525e9aae 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -1014,8 +1014,8 @@ class.  Here's a simple usage example:
"""
def test_qmp_human_info_version(self):
self.vm.launch()
-  res = self.vm.command('human-monitor-command',
-command_line='info version')
+  res = self.vm.cmd('human-monitor-command',
+command_line='info version')
self.assertRegexpMatches(res, r'^(\d+\.\d+\.\d)')
  
  To execute your test, run:

@@ -1065,15 +1065,15 @@ and hypothetical example follows:
first_machine.launch()
second_machine.launch()
  
-  first_res = first_machine.command(

+  first_res = first_machine.cmd(
'human-monitor-command',
command_line='info version')
  
-  second_res = second_machine.command(

+  second_res = second_machine.cmd(
'human-monitor-command',
command_line='info version')
  
-  third_res = self.get_vm(name='third_machine').command(

+  third_res = self.get_vm(name='third_machine').cmd(
'human-monitor-command',
command_line='info version')
  
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py

index dd1a79cb37..c4e80544bd 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -697,16 +697,16 @@ def qmp(self, cmd: str,
  self._quit_issued = True
  return ret
  
-def command(self, cmd: str,

-conv_keys: bool = True,
-**args: Any) -> QMPReturnValue:
+def cmd(self, cmd: str,
+conv_keys: bool = True,
+**args: Any) -> QMPReturnValue:
  """
  Invoke a QMP command.
  On success return the response dict.
  On failure raise an exception.
  """
  qmp_args = self._qmp_args(conv_keys, args)
-ret = self._qmp.command(cmd, **qmp_args)
+ret = self._qmp.cmd(cmd, **qmp_args)
  if cmd == 'quit':
  self._quit_issued = True
  return ret
diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index e5fa1ce9c4..22a2b5616e 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -207,7 +207,7 @@ def cmd_raw(self, name: str,
  qmp_cmd['arguments'] = args
  return self.cmd_obj(qmp_cmd)
  
-def command(self, cmd: str, **kwds: object) -> QMPReturnValue:

+def cmd(self, cmd: str, **kwds: object) -> QMPReturnValue:
  """
  Build and send a QMP 

Re: [PATCH v4 3/4] qcow2: add zoned emulation capability

2023-10-09 Thread Sam Li
Eric Blake  于2023年9月29日周五 03:17写道:
>
> On Mon, Sep 18, 2023 at 05:53:12PM +0800, Sam Li wrote:
> > By adding zone operations and zoned metadata, the zoned emulation
> > capability enables full emulation support of zoned device using
> > a qcow2 file. The zoned device metadata includes zone type,
> > zoned device state and write pointer of each zone, which is stored
> > to an array of unsigned integers.
> >
> > Each zone of a zoned device makes state transitions following
> > the zone state machine. The zone state machine mainly describes
> > five states, IMPLICIT OPEN, EXPLICIT OPEN, FULL, EMPTY and CLOSED.
> > READ ONLY and OFFLINE states will generally be affected by device
> > internal events. The operations on zones cause corresponding state
> > changing.
> >
> > Zoned devices have a limit on zone resources, which puts constraints on
> > write operations into zones.
> >
> > Signed-off-by: Sam Li 
> > ---
> >  block/qcow2.c  | 709 -
> >  block/qcow2.h  |   2 +
> >  block/trace-events |   2 +
> >  docs/interop/qcow2.txt |   6 +
> >  4 files changed, 717 insertions(+), 2 deletions(-)
>
> You may want to look at scripts/git.orderfile; putting spec changes
> (docs/*) first in your output before implementation is generally
> beneficial to reviewers.
>
> > +++ b/docs/interop/qcow2.txt
> > @@ -367,6 +367,12 @@ The fields of the zoned extension are:
> >  The maximal number of 512-byte sectors of a zone
> >  append request that can be issued to the device.
> >
> > +  36 - 43:  zonedmeta_offset
> > +The offset of zoned metadata structure in the file in 
> > bytes.
>
> For the spec to be useful, you also need to add a section describing
> the layout of the zoned metadata structure actually is.
>
> > +
> > +  44 - 51:  zonedmeta_size
> > +The size of zoned metadata in bytes.
> > +
>
> Can the zoned metadata structure ever occupy more than 4G, or can this
> field be sized at 4 bytes instead of 8?

The zoned metadata is the write pointers of all zones. The size of it
is nr_zones (uint32_t) * write_pointer size (uint64_t). So it will not
occupy more than 4G. But it still need more than 4 bytes.

>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.
> Virtualization:  qemu.org | libguestfs.org
>