[Qemu-devel] [RFC PATCH 0/5] expose negotiated virtio features in r/o properties

[Qemu-devel] [RFC PATCH 0/5] expose negotiated virtio features in r/o properties

[Roman Kagan]

This series is an attempt to make virtio features acknowledged by the
guest visible as read-only QOM properties.  One potential usecase of
this is debugging; another is when the upper layer needs to do something
only when/if the guest has acknowledged the support for a feature (e.g.
hot-plug a VFIO device once the guest claims VIRTIO_NET_F_STANDBY
support).

Being an RFC, it's incomplete and fails checkpatch, but I'd be intersted
to know if the approach is sane and worthwhile before I invest more time
in it.

Roman Kagan (5):
  qom: preserve get/set presence in aliased properties
  qmp: further consolidate listing of device and object properties
  qdev-properties: add r/o 64bit bitfield property
  virtio: drop DEFINE_VIRTIO_COMMON_FEATURES
  virtio: expose negotiated features in r/o properties

 include/hw/qdev-properties.h    |  9 ++++
 include/hw/virtio/virtio-scsi.h |  2 +-
 include/hw/virtio/virtio.h      | 18 +++----
 hw/char/virtio-serial-bus.c     |  6 ++-
 hw/core/qdev-properties.c       |  9 +++-
 hw/net/virtio-net.c             | 88 +++++++++++++++++--------------
 hw/scsi/virtio-scsi.c           |  4 +-
 hw/virtio/virtio.c              | 11 +++-
 qmp.c                           | 92 ++++++++-------------------------
 qom/object.c                    |  4 +-
 10 files changed, 113 insertions(+), 130 deletions(-)

-- 
2.19.2

[Qemu-devel] [RFC PATCH 1/5] qom: preserve get/set presence in aliased properties

[Roman Kagan]

Usually in order to tell if a property is read-only, write-only, or
read-write, one has to look at whether it has .get or .set methods.

However, property aliases are always defined with both, and it's not
until the call to the getter or setter when the support for the
corresponding operation can be found out.

To make it easier to determine if an operation is supported for an alias
property, only assign it getter and setter if the target property has
the corresponding method.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 qom/object.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 17921c0a71..b362ebab19 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -2382,8 +2382,8 @@ void object_property_add_alias(Object *obj, const char *name,
     prop->target_name = g_strdup(target_name);
 
     op = object_property_add(obj, name, prop_type,
-                             property_get_alias,
-                             property_set_alias,
+                             target_prop->get ? property_get_alias : NULL,
+                             target_prop->set ? property_set_alias : NULL,
                              property_release_alias,
                              prop, &local_err);
     if (local_err) {
-- 
2.19.2

[Qemu-devel] [RFC PATCH 2/5] qmp: further consolidate listing of device and object properties

[Roman Kagan]

Take the approach of commit 35f63767dc77d85bebff6c6565aceaf74023776a
"qmp: Merge ObjectPropertyInfo and DevicePropertyInfo" one step further:
drop device property-specific code from qmp_device_list_properties and
consolidate the resulting common part with qmp_qom_list_properties.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 qmp.c | 92 ++++++++++++++---------------------------------------------
 1 file changed, 22 insertions(+), 70 deletions(-)

diff --git a/qmp.c b/qmp.c
index e7c0a2fd60..673dfa72ce 100644
--- a/qmp.c
+++ b/qmp.c
@@ -430,56 +430,22 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
     return ret;
 }
 
-/* Return a DevicePropertyInfo for a qdev property.
- *
- * If a qdev property with the given name does not exist, use the given default
- * type.  If the qdev property info should not be shown, return NULL.
- *
- * The caller must free the return value.
- */
-static ObjectPropertyInfo *make_device_property_info(ObjectClass *klass,
-                                                  const char *name,
-                                                  const char *default_type,
-                                                  const char *description)
+static void push_property_info(ObjectPropertyInfoList **prop_list,
+                               ObjectProperty *prop)
 {
     ObjectPropertyInfo *info;
-    Property *prop;
-
-    do {
-        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
-            if (strcmp(name, prop->name) != 0) {
-                continue;
-            }
-
-            /*
-             * TODO Properties without a parser are just for dirty hacks.
-             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
-             * for removal.  This conditional should be removed along with
-             * it.
-             */
-            if (!prop->info->set && !prop->info->create) {
-                return NULL;           /* no way to set it, don't show */
-            }
-
-            info = g_malloc0(sizeof(*info));
-            info->name = g_strdup(prop->name);
-            info->type = default_type ? g_strdup(default_type)
-                                      : g_strdup(prop->info->name);
-            info->has_description = !!prop->info->description;
-            info->description = g_strdup(prop->info->description);
-            return info;
-        }
-        klass = object_class_get_parent(klass);
-    } while (klass != object_class_by_name(TYPE_DEVICE));
-
-    /* Not a qdev property, use the default type */
-    info = g_malloc0(sizeof(*info));
-    info->name = g_strdup(name);
-    info->type = g_strdup(default_type);
-    info->has_description = !!description;
-    info->description = g_strdup(description);
-
-    return info;
+    ObjectPropertyInfoList *entry;
+
+    info = g_new0(ObjectPropertyInfo, 1);
+    info->name = g_strdup(prop->name);
+    info->type = g_strdup(prop->type);
+    info->has_description = !!prop->description;
+    info->description = g_strdup(prop->description);
+
+    entry = g_new0(ObjectPropertyInfoList, 1);
+    entry->value = info;
+    entry->next = *prop_list;
+    *prop_list = entry;
 }
 
 ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
@@ -514,9 +480,6 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
 
     object_property_iter_init(&iter, obj);
     while ((prop = object_property_iter_next(&iter))) {
-        ObjectPropertyInfo *info;
-        ObjectPropertyInfoList *entry;
-
         /* Skip Object and DeviceState properties */
         if (strcmp(prop->name, "type") == 0 ||
             strcmp(prop->name, "realized") == 0 ||
@@ -533,16 +496,12 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
             continue;
         }
 
-        info = make_device_property_info(klass, prop->name, prop->type,
-                                         prop->description);
-        if (!info) {
+        /* Skip readonly properties. */
+        if (!prop->set) {
             continue;
         }
 
-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = prop_list;
-        prop_list = entry;
+        push_property_info(&prop_list, prop);
     }
 
     object_unref(obj);
@@ -579,19 +538,12 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
         object_property_iter_init(&iter, obj);
     }
     while ((prop = object_property_iter_next(&iter))) {
-        ObjectPropertyInfo *info;
-        ObjectPropertyInfoList *entry;
+        /* Skip readonly properties. */
+        if (!prop->set) {
+            continue;
+        }
 
-        info = g_malloc0(sizeof(*info));
-        info->name = g_strdup(prop->name);
-        info->type = g_strdup(prop->type);
-        info->has_description = !!prop->description;
-        info->description = g_strdup(prop->description);
-
-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = prop_list;
-        prop_list = entry;
+        push_property_info(&prop_list, prop);
     }
 
     object_unref(obj);
-- 
2.19.2

[Qemu-devel] [RFC PATCH 3/5] qdev-properties: add r/o 64bit bitfield property

[Roman Kagan]

Add a version 64bit bitfield property with no setter, useful for
introspecting the device state without being able to modify it.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 include/hw/qdev-properties.h | 9 +++++++++
 hw/core/qdev-properties.c    | 9 ++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 3ab9cd2eb6..24df135ff8 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -9,6 +9,7 @@
 
 extern const PropertyInfo qdev_prop_bit;
 extern const PropertyInfo qdev_prop_bit64;
+extern const PropertyInfo qdev_prop_bit64_ro;
 extern const PropertyInfo qdev_prop_bool;
 extern const PropertyInfo qdev_prop_uint8;
 extern const PropertyInfo qdev_prop_uint16;
@@ -96,6 +97,14 @@ extern const PropertyInfo qdev_prop_off_auto_pcibar;
         .defval.u  = (bool)_defval,                                     \
         }
 
+#define DEFINE_PROP_BIT64_RO(_name, _state, _field, _bit) {             \
+        .name      = (_name),                                           \
+        .info      = &(qdev_prop_bit64_ro),                             \
+        .bitnr    = (_bit),                                             \
+        .offset    = offsetof(_state, _field)                           \
+            + type_check(uint64_t, typeof_field(_state, _field)),       \
+        }
+
 #define DEFINE_PROP_BOOL(_name, _state, _field, _defval) {       \
         .name      = (_name),                                    \
         .info      = &(qdev_prop_bool),                          \
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index bd84c4ea4c..bb9bd48e5c 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -146,7 +146,8 @@ const PropertyInfo qdev_prop_bit = {
 
 static uint64_t qdev_get_prop_mask64(Property *prop)
 {
-    assert(prop->info == &qdev_prop_bit64);
+    assert(prop->info == &qdev_prop_bit64 ||
+           prop->info == &qdev_prop_bit64_ro);
     return 0x1ull << prop->bitnr;
 }
 
@@ -201,6 +202,12 @@ const PropertyInfo qdev_prop_bit64 = {
     .set_default_value = set_default_value_bool,
 };
 
+const PropertyInfo qdev_prop_bit64_ro = {
+    .name  = "bool",
+    .description = "on/off",
+    .get   = prop_get_bit64,
+};
+
 /* --- bool --- */
 
 static void get_bool(Object *obj, Visitor *v, const char *name, void *opaque,
-- 
2.19.2

[Qemu-devel] [RFC PATCH 4/5] virtio: drop DEFINE_VIRTIO_COMMON_FEATURES

[Roman Kagan]

This macro is only used in one place so seems to be unnecessary.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 include/hw/virtio/virtio.h | 12 ------------
 hw/virtio/virtio.c         | 11 ++++++++++-
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9c1fa07d6d..cea356efed 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -254,18 +254,6 @@ typedef struct virtio_input_conf virtio_input_conf;
 typedef struct VirtIOSCSIConf VirtIOSCSIConf;
 typedef struct VirtIORNGConf VirtIORNGConf;
 
-#define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
-    DEFINE_PROP_BIT64("indirect_desc", _state, _field,    \
-                      VIRTIO_RING_F_INDIRECT_DESC, true), \
-    DEFINE_PROP_BIT64("event_idx", _state, _field,        \
-                      VIRTIO_RING_F_EVENT_IDX, true),     \
-    DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
-                      VIRTIO_F_NOTIFY_ON_EMPTY, true), \
-    DEFINE_PROP_BIT64("any_layout", _state, _field, \
-                      VIRTIO_F_ANY_LAYOUT, true), \
-    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
-                      VIRTIO_F_IOMMU_PLATFORM, false)
-
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 22bd1ac34e..99d396c516 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2601,7 +2601,16 @@ static void virtio_device_instance_finalize(Object *obj)
 }
 
 static Property virtio_properties[] = {
-    DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
+    DEFINE_PROP_BIT64("indirect_desc", VirtIODevice, host_features,
+                      VIRTIO_RING_F_INDIRECT_DESC, true),
+    DEFINE_PROP_BIT64("event_idx", VirtIODevice, host_features,
+                      VIRTIO_RING_F_EVENT_IDX, true),
+    DEFINE_PROP_BIT64("notify_on_empty", VirtIODevice, host_features,
+                      VIRTIO_F_NOTIFY_ON_EMPTY, true),
+    DEFINE_PROP_BIT64("any_layout", VirtIODevice, host_features,
+                      VIRTIO_F_ANY_LAYOUT, true),
+    DEFINE_PROP_BIT64("iommu_platform", VirtIODevice, host_features,
+                      VIRTIO_F_IOMMU_PLATFORM, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.19.2

[Qemu-devel] [RFC PATCH 5/5] virtio: expose negotiated features in r/o properties

[Roman Kagan]

Make virtio features acknowledged by the guest visible through QOM as
read-only properties.  One potential usecase of this is debugging;
another is when the upper layer needs to do something only when/if the
guest has acknowledged the support for a feature (e.g. hot-plug a VFIO
device once the guest claims VIRTIO_NET_F_STANDBY support).

Since most of the feature bits already have associated properties for
host_features, reuse those definitions by creating a new macro that
combines the original definition for the host_features bit property and
a definition of a read-only guest_features bit property with the same
name prefixed with "negotiated-".  For the features which have no
associated host_features bit property, only the latter is defined.

Note #1: the macro is somewhat fragile as it produces two values
separated by a comma, to be used for initializing consecutive elements
in an array
Note #2: due to note #1, it fails checkpatch.
Note #3: it is also somewhat fragile as it assumes its first argument to
be a string literal
Note #4: for RFC purposes I only converted some virtio devices.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 include/hw/virtio/virtio-scsi.h |  2 +-
 include/hw/virtio/virtio.h      |  8 +++
 hw/char/virtio-serial-bus.c     |  6 ++-
 hw/net/virtio-net.c             | 88 ++++++++++++++++++---------------
 hw/scsi/virtio-scsi.c           |  4 +-
 hw/virtio/virtio.c              | 20 ++++----
 6 files changed, 73 insertions(+), 55 deletions(-)

diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 4c0bcdb788..9b412bd2c3 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -86,7 +86,7 @@ typedef struct VirtIOSCSI {
     bool dataplane_starting;
     bool dataplane_stopping;
     bool dataplane_fenced;
-    uint32_t host_features;
+    uint64_t host_features;
 } VirtIOSCSI;
 
 typedef struct VirtIOSCSIReq {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index cea356efed..a4690e6176 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -254,6 +254,14 @@ typedef struct virtio_input_conf virtio_input_conf;
 typedef struct VirtIOSCSIConf VirtIOSCSIConf;
 typedef struct VirtIORNGConf VirtIORNGConf;
 
+#define DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED(_name, _bit)     \
+    DEFINE_PROP_BIT64_RO("negotiated-" _name, VirtIODevice,   \
+                         guest_features, _bit)
+
+#define DEFINE_VIRTIO_FEATURE_BIT(_name, _state, _field, _bit, _defval) \
+    DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval),            \
+    DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED(_name, _bit)
+
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 04e3ebe352..07bf729891 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1154,8 +1154,10 @@ static const VMStateDescription vmstate_virtio_console = {
 static Property virtio_serial_properties[] = {
     DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports,
                                                   31),
-    DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
-                      VIRTIO_CONSOLE_F_EMERG_WRITE, true),
+    DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED("multiport",
+                                         VIRTIO_CONSOLE_F_MULTIPORT),
+    DEFINE_VIRTIO_FEATURE_BIT("emergency-write", VirtIOSerial, host_features,
+                              VIRTIO_CONSOLE_F_EMERG_WRITE, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 385b1a03e9..d4df3394ee 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2144,46 +2144,54 @@ static const VMStateDescription vmstate_virtio_net = {
 };
 
 static Property virtio_net_properties[] = {
-    DEFINE_PROP_BIT64("csum", VirtIONet, host_features,
-                    VIRTIO_NET_F_CSUM, true),
-    DEFINE_PROP_BIT64("guest_csum", VirtIONet, host_features,
-                    VIRTIO_NET_F_GUEST_CSUM, true),
-    DEFINE_PROP_BIT64("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
-    DEFINE_PROP_BIT64("guest_tso4", VirtIONet, host_features,
-                    VIRTIO_NET_F_GUEST_TSO4, true),
-    DEFINE_PROP_BIT64("guest_tso6", VirtIONet, host_features,
-                    VIRTIO_NET_F_GUEST_TSO6, true),
-    DEFINE_PROP_BIT64("guest_ecn", VirtIONet, host_features,
-                    VIRTIO_NET_F_GUEST_ECN, true),
-    DEFINE_PROP_BIT64("guest_ufo", VirtIONet, host_features,
-                    VIRTIO_NET_F_GUEST_UFO, true),
-    DEFINE_PROP_BIT64("guest_announce", VirtIONet, host_features,
-                    VIRTIO_NET_F_GUEST_ANNOUNCE, true),
-    DEFINE_PROP_BIT64("host_tso4", VirtIONet, host_features,
-                    VIRTIO_NET_F_HOST_TSO4, true),
-    DEFINE_PROP_BIT64("host_tso6", VirtIONet, host_features,
-                    VIRTIO_NET_F_HOST_TSO6, true),
-    DEFINE_PROP_BIT64("host_ecn", VirtIONet, host_features,
-                    VIRTIO_NET_F_HOST_ECN, true),
-    DEFINE_PROP_BIT64("host_ufo", VirtIONet, host_features,
-                    VIRTIO_NET_F_HOST_UFO, true),
-    DEFINE_PROP_BIT64("mrg_rxbuf", VirtIONet, host_features,
-                    VIRTIO_NET_F_MRG_RXBUF, true),
-    DEFINE_PROP_BIT64("status", VirtIONet, host_features,
-                    VIRTIO_NET_F_STATUS, true),
-    DEFINE_PROP_BIT64("ctrl_vq", VirtIONet, host_features,
-                    VIRTIO_NET_F_CTRL_VQ, true),
-    DEFINE_PROP_BIT64("ctrl_rx", VirtIONet, host_features,
-                    VIRTIO_NET_F_CTRL_RX, true),
-    DEFINE_PROP_BIT64("ctrl_vlan", VirtIONet, host_features,
-                    VIRTIO_NET_F_CTRL_VLAN, true),
-    DEFINE_PROP_BIT64("ctrl_rx_extra", VirtIONet, host_features,
-                    VIRTIO_NET_F_CTRL_RX_EXTRA, true),
-    DEFINE_PROP_BIT64("ctrl_mac_addr", VirtIONet, host_features,
-                    VIRTIO_NET_F_CTRL_MAC_ADDR, true),
-    DEFINE_PROP_BIT64("ctrl_guest_offloads", VirtIONet, host_features,
-                    VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true),
-    DEFINE_PROP_BIT64("mq", VirtIONet, host_features, VIRTIO_NET_F_MQ, false),
+    DEFINE_VIRTIO_FEATURE_BIT("csum", VirtIONet, host_features,
+                              VIRTIO_NET_F_CSUM, true),
+    DEFINE_VIRTIO_FEATURE_BIT("guest_csum", VirtIONet, host_features,
+                              VIRTIO_NET_F_GUEST_CSUM, true),
+    DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED("mtu",
+                                         VIRTIO_NET_F_MTU),
+    DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED("mac",
+                                         VIRTIO_NET_F_MAC),
+    DEFINE_VIRTIO_FEATURE_BIT("gso", VirtIONet, host_features,
+                              VIRTIO_NET_F_GSO, true),
+    DEFINE_VIRTIO_FEATURE_BIT("guest_tso4", VirtIONet, host_features,
+                              VIRTIO_NET_F_GUEST_TSO4, true),
+    DEFINE_VIRTIO_FEATURE_BIT("guest_tso6", VirtIONet, host_features,
+                              VIRTIO_NET_F_GUEST_TSO6, true),
+    DEFINE_VIRTIO_FEATURE_BIT("guest_ecn", VirtIONet, host_features,
+                              VIRTIO_NET_F_GUEST_ECN, true),
+    DEFINE_VIRTIO_FEATURE_BIT("guest_ufo", VirtIONet, host_features,
+                              VIRTIO_NET_F_GUEST_UFO, true),
+    DEFINE_VIRTIO_FEATURE_BIT("guest_announce", VirtIONet, host_features,
+                              VIRTIO_NET_F_GUEST_ANNOUNCE, true),
+    DEFINE_VIRTIO_FEATURE_BIT("host_tso4", VirtIONet, host_features,
+                              VIRTIO_NET_F_HOST_TSO4, true),
+    DEFINE_VIRTIO_FEATURE_BIT("host_tso6", VirtIONet, host_features,
+                              VIRTIO_NET_F_HOST_TSO6, true),
+    DEFINE_VIRTIO_FEATURE_BIT("host_ecn", VirtIONet, host_features,
+                              VIRTIO_NET_F_HOST_ECN, true),
+    DEFINE_VIRTIO_FEATURE_BIT("host_ufo", VirtIONet, host_features,
+                              VIRTIO_NET_F_HOST_UFO, true),
+    DEFINE_VIRTIO_FEATURE_BIT("mrg_rxbuf", VirtIONet, host_features,
+                              VIRTIO_NET_F_MRG_RXBUF, true),
+    DEFINE_VIRTIO_FEATURE_BIT("status", VirtIONet, host_features,
+                              VIRTIO_NET_F_STATUS, true),
+    DEFINE_VIRTIO_FEATURE_BIT("ctrl_vq", VirtIONet, host_features,
+                              VIRTIO_NET_F_CTRL_VQ, true),
+    DEFINE_VIRTIO_FEATURE_BIT("ctrl_rx", VirtIONet, host_features,
+                              VIRTIO_NET_F_CTRL_RX, true),
+    DEFINE_VIRTIO_FEATURE_BIT("ctrl_vlan", VirtIONet, host_features,
+                              VIRTIO_NET_F_CTRL_VLAN, true),
+    DEFINE_VIRTIO_FEATURE_BIT("ctrl_rx_extra", VirtIONet, host_features,
+                              VIRTIO_NET_F_CTRL_RX_EXTRA, true),
+    DEFINE_VIRTIO_FEATURE_BIT("ctrl_mac_addr", VirtIONet, host_features,
+                              VIRTIO_NET_F_CTRL_MAC_ADDR, true),
+    DEFINE_VIRTIO_FEATURE_BIT("ctrl_guest_offloads", VirtIONet, host_features,
+                              VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true),
+    DEFINE_VIRTIO_FEATURE_BIT("mq", VirtIONet, host_features,
+                              VIRTIO_NET_F_MQ, false),
+    DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED("speed_duplex",
+                                         VIRTIO_NET_F_SPEED_DUPLEX),
     DEFINE_NIC_PROPERTIES(VirtIONet, nic_conf),
     DEFINE_PROP_UINT32("x-txtimer", VirtIONet, net_conf.txtimer,
                        TX_TIMER_INTERVAL),
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3aa99717e2..6272faa180 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -923,9 +923,9 @@ static Property virtio_scsi_properties[] = {
                                                   0xFFFF),
     DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
                                                   128),
-    DEFINE_PROP_BIT("hotplug", VirtIOSCSI, host_features,
+    DEFINE_VIRTIO_FEATURE_BIT("hotplug", VirtIOSCSI, host_features,
                                            VIRTIO_SCSI_F_HOTPLUG, true),
-    DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,
+    DEFINE_VIRTIO_FEATURE_BIT("param_change", VirtIOSCSI, host_features,
                                                 VIRTIO_SCSI_F_CHANGE, true),
     DEFINE_PROP_LINK("iothread", VirtIOSCSI, parent_obj.conf.iothread,
                      TYPE_IOTHREAD, IOThread *),
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 99d396c516..2020327eb8 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2601,16 +2601,16 @@ static void virtio_device_instance_finalize(Object *obj)
 }
 
 static Property virtio_properties[] = {
-    DEFINE_PROP_BIT64("indirect_desc", VirtIODevice, host_features,
-                      VIRTIO_RING_F_INDIRECT_DESC, true),
-    DEFINE_PROP_BIT64("event_idx", VirtIODevice, host_features,
-                      VIRTIO_RING_F_EVENT_IDX, true),
-    DEFINE_PROP_BIT64("notify_on_empty", VirtIODevice, host_features,
-                      VIRTIO_F_NOTIFY_ON_EMPTY, true),
-    DEFINE_PROP_BIT64("any_layout", VirtIODevice, host_features,
-                      VIRTIO_F_ANY_LAYOUT, true),
-    DEFINE_PROP_BIT64("iommu_platform", VirtIODevice, host_features,
-                      VIRTIO_F_IOMMU_PLATFORM, false),
+    DEFINE_VIRTIO_FEATURE_BIT("indirect_desc", VirtIODevice, host_features,
+                              VIRTIO_RING_F_INDIRECT_DESC, true),
+    DEFINE_VIRTIO_FEATURE_BIT("event_idx", VirtIODevice, host_features,
+                              VIRTIO_RING_F_EVENT_IDX, true),
+    DEFINE_VIRTIO_FEATURE_BIT("notify_on_empty", VirtIODevice, host_features,
+                              VIRTIO_F_NOTIFY_ON_EMPTY, true),
+    DEFINE_VIRTIO_FEATURE_BIT("any_layout", VirtIODevice, host_features,
+                              VIRTIO_F_ANY_LAYOUT, true),
+    DEFINE_VIRTIO_FEATURE_BIT("iommu_platform", VirtIODevice, host_features,
+                              VIRTIO_F_IOMMU_PLATFORM, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.19.2

Re: [Qemu-devel] [RFC PATCH 0/5] expose negotiated virtio features in r/o properties

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181214165657.749-1-rkagan@virtuozzo.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20181214165657.749-1-rkagan@virtuozzo.com
Type: series
Subject: [Qemu-devel] [RFC PATCH 0/5] expose negotiated virtio features in r/o properties

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
a786b7b virtio: expose negotiated features in r/o properties
0c6bf38 virtio: drop DEFINE_VIRTIO_COMMON_FEATURES
32e832d qdev-properties: add r/o 64bit bitfield property
86475cf qmp: further consolidate listing of device and object properties
09f1169 qom: preserve get/set presence in aliased properties

=== OUTPUT BEGIN ===
Checking PATCH 1/5: qom: preserve get/set presence in aliased properties...
Checking PATCH 2/5: qmp: further consolidate listing of device and object properties...
Checking PATCH 3/5: qdev-properties: add r/o 64bit bitfield property...
Checking PATCH 4/5: virtio: drop DEFINE_VIRTIO_COMMON_FEATURES...
Checking PATCH 5/5: virtio: expose negotiated features in r/o properties...
ERROR: Macros with complex values should be enclosed in parenthesis
#218: FILE: include/hw/virtio/virtio.h:261:
+#define DEFINE_VIRTIO_FEATURE_BIT(_name, _state, _field, _bit, _defval) \
+    DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval),            \
+    DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED(_name, _bit)

total: 1 errors, 0 warnings, 165 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181214165657.749-1-rkagan@virtuozzo.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com