Re: [PATCH v9 16/16] hw/vmapple/virtio-blk: Replace variant types with property on base

Sun, 10 Nov 2024 21:10:37 -0800 

On 2024/11/11 6:55, Phil Dennis-Jordan wrote:

This reduces the type hierarchy of the vmapple-virtio-blk-pci-base
type and vmapple-virtio-root/vmapple-virtio-aux leaf types with a single
vmapple-virtio-blk-pci type which exposes a 'variant' enum property
which can be set to 'aux' or 'root'.

This change removes a bunch of device type boilerplate at the cost of
defining a new qapi enum and qdev property type.

Documentation for the vmapple machine type is also updated.
Please merge this change into "[PATCH v9 13/16] hw/vmapple/virtio-blk: Add support for apple virtio-blk". 



Signed-off-by: Phil Dennis-Jordan <p...@philjordan.eu>
---
  docs/system/arm/vmapple.rst         |  4 +--
  hw/core/qdev-properties-system.c    |  8 +++++
  hw/vmapple/virtio-blk.c             | 50 +++++++----------------------
  include/hw/qdev-properties-system.h |  6 ++++
  include/hw/vmapple/vmapple.h        |  4 +--
  qapi/virtio.json                    | 14 ++++++++
  6 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/docs/system/arm/vmapple.rst b/docs/system/arm/vmapple.rst
index 67942474b93..efe39b68dc4 100644
--- a/docs/system/arm/vmapple.rst
+++ b/docs/system/arm/vmapple.rst
@@ -56,8 +56,8 @@ to get better interactive access into the target system:
         -drive file="$DISK",if=pflash,format=raw \
         -drive file="$AUX",if=none,id=aux,format=raw \
         -drive file="$DISK",if=none,id=root,format=raw \
-       -device vmapple-virtio-aux,drive=aux \
-       -device vmapple-virtio-root,drive=root \
+       -device vmapple-virtio-blk-pci,variant=aux,drive=aux \
+       -device vmapple-virtio-blk-pci,variant=root,drive=root \
         -netdev 
user,id=net0,ipv6=off,hostfwd=tcp::2222-:22,hostfwd=tcp::5901-:5900 \
         -device virtio-net-pci,netdev=net0
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c 
index 35deef05f32..8bf8a3442d6 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -1239,3 +1239,11 @@ const PropertyInfo qdev_prop_iothread_vq_mapping_list = {
      .set = set_iothread_vq_mapping_list,
      .release = release_iothread_vq_mapping_list,
  };
+
+const PropertyInfo qdev_prop_vmapple_virtio_blk_variant = {
+    .name  = "VMAppleVirtioBlkVariant",
+    .enum_table  = &VMAppleVirtioBlkVariant_lookup,
+    .get   = qdev_propinfo_get_enum,
+    .set   = qdev_propinfo_set_enum,
+    .set_default_value = qdev_propinfo_set_default_value_enum,
+};
diff --git a/hw/vmapple/virtio-blk.c b/hw/vmapple/virtio-blk.c
index 40b33bbdac5..f883e65a5de 100644
--- a/hw/vmapple/virtio-blk.c
+++ b/hw/vmapple/virtio-blk.c
@@ -24,6 +24,7 @@
  #include "qemu/module.h"
  #include "qapi/error.h"
+#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" 
  OBJECT_DECLARE_TYPE(VMAppleVirtIOBlk, VMAppleVirtIOBlkClass, 
VMAPPLE_VIRTIO_BLK)
typedef struct VMAppleVirtIOBlkClass { 
@@ -41,14 +42,10 @@ typedef struct VMAppleVirtIOBlk {
  /*
   * vmapple-virtio-blk-pci: This extends VirtioPCIProxy.
   */
-#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci-base"
  OBJECT_DECLARE_SIMPLE_TYPE(VMAppleVirtIOBlkPCI, VMAPPLE_VIRTIO_BLK_PCI)
#define VIRTIO_BLK_T_APPLE_BARRIER 0x10000 -#define VIRTIO_APPLE_TYPE_ROOT 1 
-#define VIRTIO_APPLE_TYPE_AUX  2
-
  static bool vmapple_virtio_blk_handle_unknown_request(VirtIOBlockReq *req,
                                                        MultiReqBuffer *mrb,
                                                        uint32_t type)
@@ -109,7 +106,7 @@ static const TypeInfo vmapple_virtio_blk_info = {
  struct VMAppleVirtIOBlkPCI {
      VirtIOPCIProxy parent_obj;
      VMAppleVirtIOBlk vdev;
-    uint32_t apple_type;
+    VMAppleVirtioBlkVariant variant;
  };
@@ -119,6 +116,8 @@ static Property vmapple_virtio_blk_pci_properties[] = { 
                      VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
                         DEV_NVECTORS_UNSPECIFIED),
+    DEFINE_PROP_VMAPPLE_VIRTIO_BLK_VARIANT("variant", VMAppleVirtIOBlkPCI, 
variant,
+                                           
VM_APPLE_VIRTIO_BLK_VARIANT_UNSPECIFIED),
      DEFINE_PROP_END_OF_LIST(),
  };
@@ -128,6 +127,12 @@ static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **err 
      DeviceState *vdev = DEVICE(&dev->vdev);
      VirtIOBlkConf *conf = &dev->vdev.parent_obj.conf;
+ if (dev->variant == VM_APPLE_VIRTIO_BLK_VARIANT_UNSPECIFIED) { 
+        error_setg(errp, "Device " TYPE_VMAPPLE_VIRTIO_BLK_PCI ": must specify 
"
+                   "a variant, 'aux' or 'root'");
+        return;
+    }
+
      if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) {
          conf->num_queues = virtio_pci_optimal_num_queues(0);
      }
@@ -143,7 +148,7 @@ static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy 
*vpci_dev, Error **err
       */
      virtio_add_feature(&dev->vdev.parent_obj.host_features, 
VIRTIO_BLK_F_ZONED);
      /* Propagate the apple type down to the virtio-blk device */
-    dev->vdev.apple_type = dev->apple_type;
+    dev->vdev.apple_type = dev->variant;
      /* and spawn the virtio-blk device */
      qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
@@ -181,47 +186,16 @@ static void vmapple_virtio_blk_pci_instance_init(Object *obj) 
  }
static const VirtioPCIDeviceTypeInfo vmapple_virtio_blk_pci_info = { 
-    .base_name     = TYPE_VMAPPLE_VIRTIO_BLK_PCI,
-    .generic_name  = "vmapple-virtio-blk-pci",
+    .generic_name  = TYPE_VMAPPLE_VIRTIO_BLK_PCI,
      .instance_size = sizeof(VMAppleVirtIOBlkPCI),
      .instance_init = vmapple_virtio_blk_pci_instance_init,
      .class_init    = vmapple_virtio_blk_pci_class_init,
  };
-static void vmapple_virtio_root_instance_init(Object *obj) 
-{
-    VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj);
-
-    dev->apple_type = VIRTIO_APPLE_TYPE_ROOT;
-}
-
-static const TypeInfo vmapple_virtio_root_info = {
-    .name          = TYPE_VMAPPLE_VIRTIO_ROOT,
-    .parent        = "vmapple-virtio-blk-pci",
-    .instance_size = sizeof(VMAppleVirtIOBlkPCI),
-    .instance_init = vmapple_virtio_root_instance_init,
-};
-
-static void vmapple_virtio_aux_instance_init(Object *obj)
-{
-    VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj);
-
-    dev->apple_type = VIRTIO_APPLE_TYPE_AUX;
-}
-
-static const TypeInfo vmapple_virtio_aux_info = {
-    .name          = TYPE_VMAPPLE_VIRTIO_AUX,
-    .parent        = "vmapple-virtio-blk-pci",
-    .instance_size = sizeof(VMAppleVirtIOBlkPCI),
-    .instance_init = vmapple_virtio_aux_instance_init,
-};
-
  static void vmapple_virtio_blk_register_types(void)
  {
      type_register_static(&vmapple_virtio_blk_info);
      virtio_pci_types_register(&vmapple_virtio_blk_pci_info);
-    type_register_static(&vmapple_virtio_root_info);
-    type_register_static(&vmapple_virtio_aux_info);
  }
type_init(vmapple_virtio_blk_register_types) 
diff --git a/include/hw/qdev-properties-system.h 
b/include/hw/qdev-properties-system.h
index cdcc63056e5..6e428f3fcad 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -27,6 +27,8 @@ extern const PropertyInfo qdev_prop_pcie_link_speed;
  extern const PropertyInfo qdev_prop_pcie_link_width;
  extern const PropertyInfo qdev_prop_cpus390entitlement;
  extern const PropertyInfo qdev_prop_iothread_vq_mapping_list;
+extern const PropertyInfo qdev_prop_vmapple_virtio_blk_variant;
+


Here is an extra line.
#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \ 
      DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
@@ -94,4 +96,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list;
      DEFINE_PROP(_name, _state, _field, qdev_prop_iothread_vq_mapping_list, \
                  IOThreadVirtQueueMappingList *)
+#define DEFINE_PROP_VMAPPLE_VIRTIO_BLK_VARIANT(_n, _s, _f, _d) \ 
+    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_vmapple_virtio_blk_variant, \
+                       VMAppleVirtioBlkVariant)
+
  #endif
diff --git a/include/hw/vmapple/vmapple.h b/include/hw/vmapple/vmapple.h
index b20956e1286..9c1ad1bd8c3 100644
--- a/include/hw/vmapple/vmapple.h
+++ b/include/hw/vmapple/vmapple.h
@@ -18,8 +18,6 @@
#define TYPE_VMAPPLE_CFG "vmapple-cfg" -#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" 
-#define TYPE_VMAPPLE_VIRTIO_ROOT "vmapple-virtio-root"
-#define TYPE_VMAPPLE_VIRTIO_AUX  "vmapple-virtio-aux"
+#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci"
#endif /* HW_VMAPPLE_VMAPPLE_H */ 
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 2529c2d8b20..d351d2166ef 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -992,3 +992,17 @@
  ##
  { 'enum': 'GranuleMode',
    'data': [ '4k', '8k', '16k', '64k', 'host' ] }
+
+##
+# @VMAppleVirtioBlkVariant:
+#
+# @unspecified: The default, not a valid setting.
+#
+# @root: Block device holding the root volume
+#
+# @aux: Block device holding auxiliary data required for boot
+#
+# Since: 9.2
+##
+{ 'enum': 'VMAppleVirtioBlkVariant',
+  'data': [ 'unspecified', 'root', 'aux' ] }

Reply via email to