[PATCH] vhost-user-fs: add the "bootindex" property

Laszlo Ersek posted 1 patch 3 years, 4 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210104132401.5100-1-lersek@redhat.com
There is a newer version of this series
include/hw/virtio/vhost-user-fs.h |  1 +
hw/s390x/vhost-user-fs-ccw.c      |  2 ++
hw/virtio/vhost-user-fs-pci.c     |  2 ++
hw/virtio/vhost-user-fs.c         | 10 ++++++++++
4 files changed, 15 insertions(+)
[PATCH] vhost-user-fs: add the "bootindex" property
Posted by Laszlo Ersek 3 years, 4 months ago
virtio-fs qualifies as a bootable device minimally under OVMF, but
currently the necessary "bootindex" property is missing (fw_cfg kernel
boot notwithstanding).

Add the property. For completeness, add it to the CCW device as well;
other virtio-ccw devices seem to have "bootindex" properties too.

Example OpenFirmware device path for the "vhost-user-fs-pci" device in the
"bootorder" fw_cfg file:

  /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0

Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu-s390x@nongnu.org
Cc: virtio-fs@redhat.com
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/virtio/vhost-user-fs.h |  1 +
 hw/s390x/vhost-user-fs-ccw.c      |  2 ++
 hw/virtio/vhost-user-fs-pci.c     |  2 ++
 hw/virtio/vhost-user-fs.c         | 10 ++++++++++
 4 files changed, 15 insertions(+)

diff --git a/include/hw/virtio/vhost-user-fs.h b/include/hw/virtio/vhost-user-fs.h
index 698575277101..0d62834c2510 100644
--- a/include/hw/virtio/vhost-user-fs.h
+++ b/include/hw/virtio/vhost-user-fs.h
@@ -39,6 +39,7 @@ struct VHostUserFS {
     VhostUserState vhost_user;
     VirtQueue **req_vqs;
     VirtQueue *hiprio_vq;
+    int32_t bootindex;
 
     /*< public >*/
 };
diff --git a/hw/s390x/vhost-user-fs-ccw.c b/hw/s390x/vhost-user-fs-ccw.c
index 6c6f26929301..474e97e937b8 100644
--- a/hw/s390x/vhost-user-fs-ccw.c
+++ b/hw/s390x/vhost-user-fs-ccw.c
@@ -47,6 +47,8 @@ static void vhost_user_fs_ccw_instance_init(Object *obj)
     ccw_dev->force_revision_1 = true;
     virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
                                 TYPE_VHOST_USER_FS);
+    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
+                              "bootindex");
 }
 
 static void vhost_user_fs_ccw_class_init(ObjectClass *klass, void *data)
diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
index 8bb389bd282a..2ed8492b3fa3 100644
--- a/hw/virtio/vhost-user-fs-pci.c
+++ b/hw/virtio/vhost-user-fs-pci.c
@@ -68,6 +68,8 @@ static void vhost_user_fs_pci_instance_init(Object *obj)
 
     virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
                                 TYPE_VHOST_USER_FS);
+    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
+                              "bootindex");
 }
 
 static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index ed036ad9c13f..ac4fc34b36a2 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -22,6 +22,7 @@
 #include "qemu/error-report.h"
 #include "hw/virtio/vhost-user-fs.h"
 #include "monitor/monitor.h"
+#include "sysemu/sysemu.h"
 
 static void vuf_get_config(VirtIODevice *vdev, uint8_t *config)
 {
@@ -279,6 +280,14 @@ static Property vuf_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void vuf_instance_init(Object *obj)
+{
+    VHostUserFS *fs = VHOST_USER_FS(obj);
+
+    device_add_bootindex_property(obj, &fs->bootindex, "bootindex",
+                                  "/filesystem@0", DEVICE(obj));
+}
+
 static void vuf_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -300,6 +309,7 @@ static const TypeInfo vuf_info = {
     .name = TYPE_VHOST_USER_FS,
     .parent = TYPE_VIRTIO_DEVICE,
     .instance_size = sizeof(VHostUserFS),
+    .instance_init = vuf_instance_init,
     .class_init = vuf_class_init,
 };
 
-- 
2.19.1.3.g30247aa5d201

Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Laszlo Ersek 3 years, 4 months ago
Ping

On 01/04/21 14:24, Laszlo Ersek wrote:
> virtio-fs qualifies as a bootable device minimally under OVMF, but
> currently the necessary "bootindex" property is missing (fw_cfg kernel
> boot notwithstanding).
> 
> Add the property. For completeness, add it to the CCW device as well;
> other virtio-ccw devices seem to have "bootindex" properties too.
> 
> Example OpenFirmware device path for the "vhost-user-fs-pci" device in the
> "bootorder" fw_cfg file:
> 
>   /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Cc: virtio-fs@redhat.com
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  include/hw/virtio/vhost-user-fs.h |  1 +
>  hw/s390x/vhost-user-fs-ccw.c      |  2 ++
>  hw/virtio/vhost-user-fs-pci.c     |  2 ++
>  hw/virtio/vhost-user-fs.c         | 10 ++++++++++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/include/hw/virtio/vhost-user-fs.h b/include/hw/virtio/vhost-user-fs.h
> index 698575277101..0d62834c2510 100644
> --- a/include/hw/virtio/vhost-user-fs.h
> +++ b/include/hw/virtio/vhost-user-fs.h
> @@ -39,6 +39,7 @@ struct VHostUserFS {
>      VhostUserState vhost_user;
>      VirtQueue **req_vqs;
>      VirtQueue *hiprio_vq;
> +    int32_t bootindex;
>  
>      /*< public >*/
>  };
> diff --git a/hw/s390x/vhost-user-fs-ccw.c b/hw/s390x/vhost-user-fs-ccw.c
> index 6c6f26929301..474e97e937b8 100644
> --- a/hw/s390x/vhost-user-fs-ccw.c
> +++ b/hw/s390x/vhost-user-fs-ccw.c
> @@ -47,6 +47,8 @@ static void vhost_user_fs_ccw_instance_init(Object *obj)
>      ccw_dev->force_revision_1 = true;
>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>                                  TYPE_VHOST_USER_FS);
> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
> +                              "bootindex");
>  }
>  
>  static void vhost_user_fs_ccw_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> index 8bb389bd282a..2ed8492b3fa3 100644
> --- a/hw/virtio/vhost-user-fs-pci.c
> +++ b/hw/virtio/vhost-user-fs-pci.c
> @@ -68,6 +68,8 @@ static void vhost_user_fs_pci_instance_init(Object *obj)
>  
>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>                                  TYPE_VHOST_USER_FS);
> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
> +                              "bootindex");
>  }
>  
>  static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
> index ed036ad9c13f..ac4fc34b36a2 100644
> --- a/hw/virtio/vhost-user-fs.c
> +++ b/hw/virtio/vhost-user-fs.c
> @@ -22,6 +22,7 @@
>  #include "qemu/error-report.h"
>  #include "hw/virtio/vhost-user-fs.h"
>  #include "monitor/monitor.h"
> +#include "sysemu/sysemu.h"
>  
>  static void vuf_get_config(VirtIODevice *vdev, uint8_t *config)
>  {
> @@ -279,6 +280,14 @@ static Property vuf_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static void vuf_instance_init(Object *obj)
> +{
> +    VHostUserFS *fs = VHOST_USER_FS(obj);
> +
> +    device_add_bootindex_property(obj, &fs->bootindex, "bootindex",
> +                                  "/filesystem@0", DEVICE(obj));
> +}
> +
>  static void vuf_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -300,6 +309,7 @@ static const TypeInfo vuf_info = {
>      .name = TYPE_VHOST_USER_FS,
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(VHostUserFS),
> +    .instance_init = vuf_instance_init,
>      .class_init = vuf_class_init,
>  };
>  
> 


Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Christian Borntraeger 3 years, 4 months ago

On 04.01.21 14:24, Laszlo Ersek wrote:
> virtio-fs qualifies as a bootable device minimally under OVMF, but
> currently the necessary "bootindex" property is missing (fw_cfg kernel
> boot notwithstanding).
> 
> Add the property. For completeness, add it to the CCW device as well;
> other virtio-ccw devices seem to have "bootindex" properties too.

Currently we do not have boot support for virtiofs on s390x (ccw)
Not sure if it is better if we should add the property now or whenever
boot support is implemented. 
As of today we do have bootindex for block and net. Maybe it is better
to defer bootindex for virtio-fs-ccw until we can boot from it? In
that way management software can detect if this is bootable or not?

> 
> Example OpenFirmware device path for the "vhost-user-fs-pci" device in the
> "bootorder" fw_cfg file:
> 
>   /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Cc: virtio-fs@redhat.com
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  include/hw/virtio/vhost-user-fs.h |  1 +
>  hw/s390x/vhost-user-fs-ccw.c      |  2 ++
>  hw/virtio/vhost-user-fs-pci.c     |  2 ++
>  hw/virtio/vhost-user-fs.c         | 10 ++++++++++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/include/hw/virtio/vhost-user-fs.h b/include/hw/virtio/vhost-user-fs.h
> index 698575277101..0d62834c2510 100644
> --- a/include/hw/virtio/vhost-user-fs.h
> +++ b/include/hw/virtio/vhost-user-fs.h
> @@ -39,6 +39,7 @@ struct VHostUserFS {
>      VhostUserState vhost_user;
>      VirtQueue **req_vqs;
>      VirtQueue *hiprio_vq;
> +    int32_t bootindex;
>  
>      /*< public >*/
>  };
> diff --git a/hw/s390x/vhost-user-fs-ccw.c b/hw/s390x/vhost-user-fs-ccw.c
> index 6c6f26929301..474e97e937b8 100644
> --- a/hw/s390x/vhost-user-fs-ccw.c
> +++ b/hw/s390x/vhost-user-fs-ccw.c
> @@ -47,6 +47,8 @@ static void vhost_user_fs_ccw_instance_init(Object *obj)
>      ccw_dev->force_revision_1 = true;
>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>                                  TYPE_VHOST_USER_FS);
> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
> +                              "bootindex");
>  }
>  
>  static void vhost_user_fs_ccw_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> index 8bb389bd282a..2ed8492b3fa3 100644
> --- a/hw/virtio/vhost-user-fs-pci.c
> +++ b/hw/virtio/vhost-user-fs-pci.c
> @@ -68,6 +68,8 @@ static void vhost_user_fs_pci_instance_init(Object *obj)
>  
>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>                                  TYPE_VHOST_USER_FS);
> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
> +                              "bootindex");
>  }
>  
>  static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
> index ed036ad9c13f..ac4fc34b36a2 100644
> --- a/hw/virtio/vhost-user-fs.c
> +++ b/hw/virtio/vhost-user-fs.c
> @@ -22,6 +22,7 @@
>  #include "qemu/error-report.h"
>  #include "hw/virtio/vhost-user-fs.h"
>  #include "monitor/monitor.h"
> +#include "sysemu/sysemu.h"
>  
>  static void vuf_get_config(VirtIODevice *vdev, uint8_t *config)
>  {
> @@ -279,6 +280,14 @@ static Property vuf_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static void vuf_instance_init(Object *obj)
> +{
> +    VHostUserFS *fs = VHOST_USER_FS(obj);
> +
> +    device_add_bootindex_property(obj, &fs->bootindex, "bootindex",
> +                                  "/filesystem@0", DEVICE(obj));
> +}
> +
>  static void vuf_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -300,6 +309,7 @@ static const TypeInfo vuf_info = {
>      .name = TYPE_VHOST_USER_FS,
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(VHostUserFS),
> +    .instance_init = vuf_instance_init,
>      .class_init = vuf_class_init,
>  };
>  
> 

Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Laszlo Ersek 3 years, 4 months ago
On 01/11/21 10:46, Christian Borntraeger wrote:
> 
> 
> On 04.01.21 14:24, Laszlo Ersek wrote:
>> virtio-fs qualifies as a bootable device minimally under OVMF, but
>> currently the necessary "bootindex" property is missing (fw_cfg kernel
>> boot notwithstanding).
>>
>> Add the property. For completeness, add it to the CCW device as well;
>> other virtio-ccw devices seem to have "bootindex" properties too.
> 
> Currently we do not have boot support for virtiofs on s390x (ccw)
> Not sure if it is better if we should add the property now or whenever
> boot support is implemented. 
> As of today we do have bootindex for block and net. Maybe it is better
> to defer bootindex for virtio-fs-ccw until we can boot from it? In
> that way management software can detect if this is bootable or not?

Sure, I can drop the "vhost-user-fs-ccw.c hunk".

(I guess I could even lift the property itself (replacing the alias) to
"vhost-user-fs-pci.c", for now -- what's everyone's take on that?)

Thanks
Laszlo

> 
>>
>> Example OpenFirmware device path for the "vhost-user-fs-pci" device in the
>> "bootorder" fw_cfg file:
>>
>>   /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0
>>
>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Cc: Cornelia Huck <cohuck@redhat.com>
>> Cc: Halil Pasic <pasic@linux.ibm.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Cc: qemu-s390x@nongnu.org
>> Cc: virtio-fs@redhat.com
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  include/hw/virtio/vhost-user-fs.h |  1 +
>>  hw/s390x/vhost-user-fs-ccw.c      |  2 ++
>>  hw/virtio/vhost-user-fs-pci.c     |  2 ++
>>  hw/virtio/vhost-user-fs.c         | 10 ++++++++++
>>  4 files changed, 15 insertions(+)
>>
>> diff --git a/include/hw/virtio/vhost-user-fs.h b/include/hw/virtio/vhost-user-fs.h
>> index 698575277101..0d62834c2510 100644
>> --- a/include/hw/virtio/vhost-user-fs.h
>> +++ b/include/hw/virtio/vhost-user-fs.h
>> @@ -39,6 +39,7 @@ struct VHostUserFS {
>>      VhostUserState vhost_user;
>>      VirtQueue **req_vqs;
>>      VirtQueue *hiprio_vq;
>> +    int32_t bootindex;
>>  
>>      /*< public >*/
>>  };
>> diff --git a/hw/s390x/vhost-user-fs-ccw.c b/hw/s390x/vhost-user-fs-ccw.c
>> index 6c6f26929301..474e97e937b8 100644
>> --- a/hw/s390x/vhost-user-fs-ccw.c
>> +++ b/hw/s390x/vhost-user-fs-ccw.c
>> @@ -47,6 +47,8 @@ static void vhost_user_fs_ccw_instance_init(Object *obj)
>>      ccw_dev->force_revision_1 = true;
>>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>>                                  TYPE_VHOST_USER_FS);
>> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
>> +                              "bootindex");
>>  }
>>  
>>  static void vhost_user_fs_ccw_class_init(ObjectClass *klass, void *data)
>> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
>> index 8bb389bd282a..2ed8492b3fa3 100644
>> --- a/hw/virtio/vhost-user-fs-pci.c
>> +++ b/hw/virtio/vhost-user-fs-pci.c
>> @@ -68,6 +68,8 @@ static void vhost_user_fs_pci_instance_init(Object *obj)
>>  
>>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>>                                  TYPE_VHOST_USER_FS);
>> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
>> +                              "bootindex");
>>  }
>>  
>>  static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
>> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
>> index ed036ad9c13f..ac4fc34b36a2 100644
>> --- a/hw/virtio/vhost-user-fs.c
>> +++ b/hw/virtio/vhost-user-fs.c
>> @@ -22,6 +22,7 @@
>>  #include "qemu/error-report.h"
>>  #include "hw/virtio/vhost-user-fs.h"
>>  #include "monitor/monitor.h"
>> +#include "sysemu/sysemu.h"
>>  
>>  static void vuf_get_config(VirtIODevice *vdev, uint8_t *config)
>>  {
>> @@ -279,6 +280,14 @@ static Property vuf_properties[] = {
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> +static void vuf_instance_init(Object *obj)
>> +{
>> +    VHostUserFS *fs = VHOST_USER_FS(obj);
>> +
>> +    device_add_bootindex_property(obj, &fs->bootindex, "bootindex",
>> +                                  "/filesystem@0", DEVICE(obj));
>> +}
>> +
>>  static void vuf_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -300,6 +309,7 @@ static const TypeInfo vuf_info = {
>>      .name = TYPE_VHOST_USER_FS,
>>      .parent = TYPE_VIRTIO_DEVICE,
>>      .instance_size = sizeof(VHostUserFS),
>> +    .instance_init = vuf_instance_init,
>>      .class_init = vuf_class_init,
>>  };
>>  
>>
> 


Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Christian Borntraeger 3 years, 4 months ago

On 11.01.21 11:05, Laszlo Ersek wrote:
> On 01/11/21 10:46, Christian Borntraeger wrote:
>>
>>
>> On 04.01.21 14:24, Laszlo Ersek wrote:
>>> virtio-fs qualifies as a bootable device minimally under OVMF, but
>>> currently the necessary "bootindex" property is missing (fw_cfg kernel
>>> boot notwithstanding).
>>>
>>> Add the property. For completeness, add it to the CCW device as well;
>>> other virtio-ccw devices seem to have "bootindex" properties too.
>>
>> Currently we do not have boot support for virtiofs on s390x (ccw)
>> Not sure if it is better if we should add the property now or whenever
>> boot support is implemented. 
>> As of today we do have bootindex for block and net. Maybe it is better
>> to defer bootindex for virtio-fs-ccw until we can boot from it? In
>> that way management software can detect if this is bootable or not?
> 
> Sure, I can drop the "vhost-user-fs-ccw.c hunk".
> 
> (I guess I could even lift the property itself (replacing the alias) to
> "vhost-user-fs-pci.c", for now -- what's everyone's take on that?)

Keep it as is. This would simplify a later addition to ccw.

Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Laszlo Ersek 3 years, 4 months ago
On 01/11/21 11:07, Christian Borntraeger wrote:
> 
> 
> On 11.01.21 11:05, Laszlo Ersek wrote:
>> On 01/11/21 10:46, Christian Borntraeger wrote:
>>>
>>>
>>> On 04.01.21 14:24, Laszlo Ersek wrote:
>>>> virtio-fs qualifies as a bootable device minimally under OVMF, but
>>>> currently the necessary "bootindex" property is missing (fw_cfg kernel
>>>> boot notwithstanding).
>>>>
>>>> Add the property. For completeness, add it to the CCW device as well;
>>>> other virtio-ccw devices seem to have "bootindex" properties too.
>>>
>>> Currently we do not have boot support for virtiofs on s390x (ccw)
>>> Not sure if it is better if we should add the property now or whenever
>>> boot support is implemented. 
>>> As of today we do have bootindex for block and net. Maybe it is better
>>> to defer bootindex for virtio-fs-ccw until we can boot from it? In
>>> that way management software can detect if this is bootable or not?
>>
>> Sure, I can drop the "vhost-user-fs-ccw.c hunk".
>>
>> (I guess I could even lift the property itself (replacing the alias) to
>> "vhost-user-fs-pci.c", for now -- what's everyone's take on that?)
> 
> Keep it as is. This would simplify a later addition to ccw.
> 

Thanks!

I'll wait for some feedback from the other reviewers, before posting v2.

Laszlo


Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Cornelia Huck 3 years, 4 months ago
On Mon,  4 Jan 2021 14:24:01 +0100
Laszlo Ersek <lersek@redhat.com> wrote:

> virtio-fs qualifies as a bootable device minimally under OVMF, but
> currently the necessary "bootindex" property is missing (fw_cfg kernel
> boot notwithstanding).
> 
> Add the property. For completeness, add it to the CCW device as well;
> other virtio-ccw devices seem to have "bootindex" properties too.
> 
> Example OpenFirmware device path for the "vhost-user-fs-pci" device in the
> "bootorder" fw_cfg file:
> 
>   /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Cc: virtio-fs@redhat.com
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  include/hw/virtio/vhost-user-fs.h |  1 +
>  hw/s390x/vhost-user-fs-ccw.c      |  2 ++
>  hw/virtio/vhost-user-fs-pci.c     |  2 ++
>  hw/virtio/vhost-user-fs.c         | 10 ++++++++++
>  4 files changed, 15 insertions(+)
> 

I agree that you should just drop the ccw hunk for now. The rest of the
patch looks good to me.


Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Laszlo Ersek 3 years, 4 months ago
On 01/11/21 17:22, Cornelia Huck wrote:
> On Mon,  4 Jan 2021 14:24:01 +0100
> Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> virtio-fs qualifies as a bootable device minimally under OVMF, but
>> currently the necessary "bootindex" property is missing (fw_cfg kernel
>> boot notwithstanding).
>>
>> Add the property. For completeness, add it to the CCW device as well;
>> other virtio-ccw devices seem to have "bootindex" properties too.
>>
>> Example OpenFirmware device path for the "vhost-user-fs-pci" device in the
>> "bootorder" fw_cfg file:
>>
>>   /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0
>>
>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Cc: Cornelia Huck <cohuck@redhat.com>
>> Cc: Halil Pasic <pasic@linux.ibm.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Cc: qemu-s390x@nongnu.org
>> Cc: virtio-fs@redhat.com
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  include/hw/virtio/vhost-user-fs.h |  1 +
>>  hw/s390x/vhost-user-fs-ccw.c      |  2 ++
>>  hw/virtio/vhost-user-fs-pci.c     |  2 ++
>>  hw/virtio/vhost-user-fs.c         | 10 ++++++++++
>>  4 files changed, 15 insertions(+)
>>
> 
> I agree that you should just drop the ccw hunk for now. The rest of the
> patch looks good to me.
> 

Thank you as well -- with the ccw hunk gone, I guess the CC list will be
shorter in v2, too :)

Dave, Stefan, Michael -- can you please comment? For the files to remain
in v2, "get_maintainer.pl" gives me:

"Dr. David Alan Gilbert" <dgilbert@redhat.com> (supporter:virtiofs)
"Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost)
"Michael S. Tsirkin" <mst@redhat.com> (supporter:virtio)
Stefan Hajnoczi <stefanha@redhat.com> (supporter:virtiofs)

Thanks
Laszlo


Re: [PATCH] vhost-user-fs: add the "bootindex" property
Posted by Dr. David Alan Gilbert 3 years, 4 months ago
* Laszlo Ersek (lersek@redhat.com) wrote:
> virtio-fs qualifies as a bootable device minimally under OVMF, but
> currently the necessary "bootindex" property is missing (fw_cfg kernel
> boot notwithstanding).
> 
> Add the property. For completeness, add it to the CCW device as well;
> other virtio-ccw devices seem to have "bootindex" properties too.
> 
> Example OpenFirmware device path for the "vhost-user-fs-pci" device in the
> "bootorder" fw_cfg file:
> 
>   /pci@i0cf8/pci-bridge@1,6/pci1af4,105a@0/filesystem@0
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Cc: virtio-fs@redhat.com
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  include/hw/virtio/vhost-user-fs.h |  1 +
>  hw/s390x/vhost-user-fs-ccw.c      |  2 ++
>  hw/virtio/vhost-user-fs-pci.c     |  2 ++
>  hw/virtio/vhost-user-fs.c         | 10 ++++++++++
>  4 files changed, 15 insertions(+)
> 

That looks reasonable; although I'm not familiar with the bootindex
system; so 

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


> diff --git a/include/hw/virtio/vhost-user-fs.h b/include/hw/virtio/vhost-user-fs.h
> index 698575277101..0d62834c2510 100644
> --- a/include/hw/virtio/vhost-user-fs.h
> +++ b/include/hw/virtio/vhost-user-fs.h
> @@ -39,6 +39,7 @@ struct VHostUserFS {
>      VhostUserState vhost_user;
>      VirtQueue **req_vqs;
>      VirtQueue *hiprio_vq;
> +    int32_t bootindex;
>  
>      /*< public >*/
>  };
> diff --git a/hw/s390x/vhost-user-fs-ccw.c b/hw/s390x/vhost-user-fs-ccw.c
> index 6c6f26929301..474e97e937b8 100644
> --- a/hw/s390x/vhost-user-fs-ccw.c
> +++ b/hw/s390x/vhost-user-fs-ccw.c
> @@ -47,6 +47,8 @@ static void vhost_user_fs_ccw_instance_init(Object *obj)
>      ccw_dev->force_revision_1 = true;
>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>                                  TYPE_VHOST_USER_FS);
> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
> +                              "bootindex");
>  }
>  
>  static void vhost_user_fs_ccw_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> index 8bb389bd282a..2ed8492b3fa3 100644
> --- a/hw/virtio/vhost-user-fs-pci.c
> +++ b/hw/virtio/vhost-user-fs-pci.c
> @@ -68,6 +68,8 @@ static void vhost_user_fs_pci_instance_init(Object *obj)
>  
>      virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>                                  TYPE_VHOST_USER_FS);
> +    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
> +                              "bootindex");
>  }
>  
>  static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
> index ed036ad9c13f..ac4fc34b36a2 100644
> --- a/hw/virtio/vhost-user-fs.c
> +++ b/hw/virtio/vhost-user-fs.c
> @@ -22,6 +22,7 @@
>  #include "qemu/error-report.h"
>  #include "hw/virtio/vhost-user-fs.h"
>  #include "monitor/monitor.h"
> +#include "sysemu/sysemu.h"
>  
>  static void vuf_get_config(VirtIODevice *vdev, uint8_t *config)
>  {
> @@ -279,6 +280,14 @@ static Property vuf_properties[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static void vuf_instance_init(Object *obj)
> +{
> +    VHostUserFS *fs = VHOST_USER_FS(obj);
> +
> +    device_add_bootindex_property(obj, &fs->bootindex, "bootindex",
> +                                  "/filesystem@0", DEVICE(obj));
> +}
> +
>  static void vuf_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -300,6 +309,7 @@ static const TypeInfo vuf_info = {
>      .name = TYPE_VHOST_USER_FS,
>      .parent = TYPE_VIRTIO_DEVICE,
>      .instance_size = sizeof(VHostUserFS),
> +    .instance_init = vuf_instance_init,
>      .class_init = vuf_class_init,
>  };
>  
> -- 
> 2.19.1.3.g30247aa5d201
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK