1. Patchew
  2. Libvirt
  3. View series

[PATCH] qemu: Support removable for scsi disk

Han Han posted 1 patch 2 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230605050729.1194184-1-hhan@redhat.com
docs/formatdomain.rst                              |  2 +-
src/conf/domain_validate.c                         |  5 +++--
src/qemu/qemu_command.c                            |  6 ++++--
...gs => disk-device-removable.x86_64-latest.args} | 14 +++++++++-----
...ice-removable.xml => disk-device-removable.xml} |  5 +++++
tests/qemuxml2argvtest.c                           |  2 +-
6 files changed, 23 insertions(+), 11 deletions(-)
rename tests/qemuxml2argvdata/{disk-usb-device-removable.x86_64-latest.args => disk-device-removable.x86_64-latest.args} (70%)
rename tests/qemuxml2argvdata/{disk-usb-device-removable.xml => disk-device-removable.xml} (80%)
[PATCH] qemu: Support removable for scsi disk
Posted by Han Han 2 years, 8 months ago 
Allow //disk/target@removable for scsi disk devices, since QEMU has support
the removable attribute for scsi-hd device from v0.14.0[1].

[1]: 419e691f8e: scsi-disk: Allow overriding SCSI INQUIRY removable bit

Signed-off-by: Han Han <hhan@redhat.com>
---
 docs/formatdomain.rst                              |  2 +-
 src/conf/domain_validate.c                         |  5 +++--
 src/qemu/qemu_command.c                            |  6 ++++--
 ...gs => disk-device-removable.x86_64-latest.args} | 14 +++++++++-----
 ...ice-removable.xml => disk-device-removable.xml} |  5 +++++
 tests/qemuxml2argvtest.c                           |  2 +-
 6 files changed, 23 insertions(+), 11 deletions(-)
 rename tests/qemuxml2argvdata/{disk-usb-device-removable.x86_64-latest.args => disk-device-removable.x86_64-latest.args} (70%)
 rename tests/qemuxml2argvdata/{disk-usb-device-removable.xml => disk-device-removable.xml} (80%)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index c3526439bf..676e4b9fd3 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3094,7 +3094,7 @@ paravirtualized driver is specified via the ``disk`` element.
    CDROM or Floppy disk), the value can be either "open" or "closed", defaults
    to "closed". NB, the value of ``tray`` could be updated while the domain is
    running. The optional attribute ``removable`` sets the removable flag for USB
-   disks, and its value can be either "on" or "off", defaulting to "off".
+   or SCSI disks, and its value can be either "on" or "off", defaulting to "off".
    The optional attribute ``rotation_rate`` sets the rotation rate of the
    storage for disks on a SCSI, IDE, or SATA bus. Values in the range 1025 to
    65534 are used to indicate rotational media speed in revolutions per minute.
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 80d6a2ffd9..e7122f6297 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -881,9 +881,10 @@ virDomainDiskDefValidate(const virDomainDef *def,
     }
 
     if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT &&
-        disk->bus != VIR_DOMAIN_DISK_BUS_USB) {
+        disk->bus != VIR_DOMAIN_DISK_BUS_USB &&
+        !(disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("removable is only valid for usb disks"));
+                       _("removable is only valid for usb or scsi disks"));
         return -1;
     }
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 72363238b8..5ec912ddee 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1781,10 +1781,12 @@ qemuBuildDiskDeviceProps(const virDomainDef *def,
         if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
             driver = "scsi-block";
         } else {
-            if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
+            if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
                 driver = "scsi-cd";
-            else
+            } else {
                 driver = "scsi-hd";
+                removable = disk->removable;
+            }
 
             /* qemu historically used the name of -drive as one of the device
              * ids in the Vital Product Data Device Identification page if
diff --git a/tests/qemuxml2argvdata/disk-usb-device-removable.x86_64-latest.args b/tests/qemuxml2argvdata/disk-device-removable.x86_64-latest.args
similarity index 70%
rename from tests/qemuxml2argvdata/disk-usb-device-removable.x86_64-latest.args
rename to tests/qemuxml2argvdata/disk-device-removable.x86_64-latest.args
index c5bfd8e8e0..106f7b9ce2 100644
--- a/tests/qemuxml2argvdata/disk-usb-device-removable.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-device-removable.x86_64-latest.args
@@ -27,13 +27,17 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -no-shutdown \
 -boot strict=on \
 -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
--blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
+-device '{"driver":"virtio-scsi-pci","id":"scsi0","bus":"pci.0","addr":"0x2"}' \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"raw","file":"libvirt-3-storage"}' \
+-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-3-format","id":"ide0-0-0","bootindex":1}' \
+-blockdev '{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
 -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw","file":"libvirt-2-storage"}' \
--device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-2-format","id":"ide0-0-0","bootindex":1}' \
--blockdev '{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-2-format","id":"usb-disk0","removable":true}' \
+-blockdev '{"driver":"file","filename":"/tmp/scsidisk.img","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
 -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
--device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-format","id":"usb-disk0","removable":true}' \
+-device '{"driver":"scsi-hd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":1,"device_id":"drive-scsi0-0-0-1","drive":"libvirt-1-format","id":"scsi0-0-0-1","removable":true}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
--device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
+-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x3"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2argvdata/disk-usb-device-removable.xml b/tests/qemuxml2argvdata/disk-device-removable.xml
similarity index 80%
rename from tests/qemuxml2argvdata/disk-usb-device-removable.xml
rename to tests/qemuxml2argvdata/disk-device-removable.xml
index 927a8e8b3e..9400c84863 100644
--- a/tests/qemuxml2argvdata/disk-usb-device-removable.xml
+++ b/tests/qemuxml2argvdata/disk-device-removable.xml
@@ -22,6 +22,11 @@
       <source file='/tmp/usbdisk.img'/>
       <target dev='sda' bus='usb' removable='on'/>
     </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/scsidisk.img'/>
+      <target dev='sdb' bus='scsi' removable='on'/>
+    </disk>
+    <controller type='scsi' index='0' model='virtio-scsi'/>
     <memballoon model='virtio'/>
   </devices>
 </domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d914d8cbea..d6a9ab865d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1227,7 +1227,7 @@ mymain(void)
     DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-device-lun-type-invalid");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-attaching-partition-nosupport");
     DO_TEST_CAPS_LATEST("disk-usb-device");
-    DO_TEST_CAPS_LATEST("disk-usb-device-removable");
+    DO_TEST_CAPS_LATEST("disk-device-removable");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-usb-pci");
     DO_TEST_CAPS_LATEST("disk-scsi");
     DO_TEST_CAPS_LATEST("disk-scsi-device-auto");
-- 
2.40.1
Re: [PATCH] qemu: Support removable for scsi disk
Posted by Han Han 2 years, 7 months ago 
ping~

On Mon, Jun 5, 2023 at 1:07 PM Han Han <hhan@redhat.com> wrote:

> Allow //disk/target@removable for scsi disk devices, since QEMU has
> support
> the removable attribute for scsi-hd device from v0.14.0[1].
>
> [1]: 419e691f8e: scsi-disk: Allow overriding SCSI INQUIRY removable bit
>
> Signed-off-by: Han Han <hhan@redhat.com>
> ---
>  docs/formatdomain.rst                              |  2 +-
>  src/conf/domain_validate.c                         |  5 +++--
>  src/qemu/qemu_command.c                            |  6 ++++--
>  ...gs => disk-device-removable.x86_64-latest.args} | 14 +++++++++-----
>  ...ice-removable.xml => disk-device-removable.xml} |  5 +++++
>  tests/qemuxml2argvtest.c                           |  2 +-
>  6 files changed, 23 insertions(+), 11 deletions(-)
>  rename
> tests/qemuxml2argvdata/{disk-usb-device-removable.x86_64-latest.args =>
> disk-device-removable.x86_64-latest.args} (70%)
>  rename tests/qemuxml2argvdata/{disk-usb-device-removable.xml =>
> disk-device-removable.xml} (80%)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index c3526439bf..676e4b9fd3 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3094,7 +3094,7 @@ paravirtualized driver is specified via the ``disk``
> element.
>     CDROM or Floppy disk), the value can be either "open" or "closed",
> defaults
>     to "closed". NB, the value of ``tray`` could be updated while the
> domain is
>     running. The optional attribute ``removable`` sets the removable flag
> for USB
> -   disks, and its value can be either "on" or "off", defaulting to "off".
> +   or SCSI disks, and its value can be either "on" or "off", defaulting
> to "off".
>     The optional attribute ``rotation_rate`` sets the rotation rate of the
>     storage for disks on a SCSI, IDE, or SATA bus. Values in the range
> 1025 to
>     65534 are used to indicate rotational media speed in revolutions per
> minute.
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index 80d6a2ffd9..e7122f6297 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -881,9 +881,10 @@ virDomainDiskDefValidate(const virDomainDef *def,
>      }
>
>      if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT &&
> -        disk->bus != VIR_DOMAIN_DISK_BUS_USB) {
> +        disk->bus != VIR_DOMAIN_DISK_BUS_USB &&
> +        !(disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && disk->device ==
> VIR_DOMAIN_DISK_DEVICE_DISK)) {
>          virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("removable is only valid for usb disks"));
> +                       _("removable is only valid for usb or scsi
> disks"));
>          return -1;
>      }
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 72363238b8..5ec912ddee 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1781,10 +1781,12 @@ qemuBuildDiskDeviceProps(const virDomainDef *def,
>          if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
>              driver = "scsi-block";
>          } else {
> -            if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> +            if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
>                  driver = "scsi-cd";
> -            else
> +            } else {
>                  driver = "scsi-hd";
> +                removable = disk->removable;
> +            }
>
>              /* qemu historically used the name of -drive as one of the
> device
>               * ids in the Vital Product Data Device Identification page if
> diff --git
> a/tests/qemuxml2argvdata/disk-usb-device-removable.x86_64-latest.args
> b/tests/qemuxml2argvdata/disk-device-removable.x86_64-latest.args
> similarity index 70%
> rename from
> tests/qemuxml2argvdata/disk-usb-device-removable.x86_64-latest.args
> rename to tests/qemuxml2argvdata/disk-device-removable.x86_64-latest.args
> index c5bfd8e8e0..106f7b9ce2 100644
> --- a/tests/qemuxml2argvdata/disk-usb-device-removable.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/disk-device-removable.x86_64-latest.args
> @@ -27,13 +27,17 @@
> XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
>  -no-shutdown \
>  -boot strict=on \
>  -device
> '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
> --blockdev
> '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
> \
> +-device
> '{"driver":"virtio-scsi-pci","id":"scsi0","bus":"pci.0","addr":"0x2"}' \
> +-blockdev
> '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}'
> \
> +-blockdev
> '{"node-name":"libvirt-3-format","read-only":false,"driver":"raw","file":"libvirt-3-storage"}'
> \
> +-device
> '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-3-format","id":"ide0-0-0","bootindex":1}'
> \
> +-blockdev
> '{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
> \
>  -blockdev
> '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw","file":"libvirt-2-storage"}'
> \
> --device
> '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-2-format","id":"ide0-0-0","bootindex":1}'
> \
> --blockdev
> '{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
> \
> +-device
> '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-2-format","id":"usb-disk0","removable":true}'
> \
> +-blockdev
> '{"driver":"file","filename":"/tmp/scsidisk.img","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
> \
>  -blockdev
> '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}'
> \
> --device
> '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-format","id":"usb-disk0","removable":true}'
> \
> +-device
> '{"driver":"scsi-hd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":1,"device_id":"drive-scsi0-0-0-1","drive":"libvirt-1-format","id":"scsi0-0-0-1","removable":true}'
> \
>  -audiodev '{"id":"audio1","driver":"none"}' \
> --device
> '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}'
> \
> +-device
> '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x3"}'
> \
>  -sandbox
> on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
>  -msg timestamp=on
> diff --git a/tests/qemuxml2argvdata/disk-usb-device-removable.xml
> b/tests/qemuxml2argvdata/disk-device-removable.xml
> similarity index 80%
> rename from tests/qemuxml2argvdata/disk-usb-device-removable.xml
> rename to tests/qemuxml2argvdata/disk-device-removable.xml
> index 927a8e8b3e..9400c84863 100644
> --- a/tests/qemuxml2argvdata/disk-usb-device-removable.xml
> +++ b/tests/qemuxml2argvdata/disk-device-removable.xml
> @@ -22,6 +22,11 @@
>        <source file='/tmp/usbdisk.img'/>
>        <target dev='sda' bus='usb' removable='on'/>
>      </disk>
> +    <disk type='file' device='disk'>
> +      <source file='/tmp/scsidisk.img'/>
> +      <target dev='sdb' bus='scsi' removable='on'/>
> +    </disk>
> +    <controller type='scsi' index='0' model='virtio-scsi'/>
>      <memballoon model='virtio'/>
>    </devices>
>  </domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index d914d8cbea..d6a9ab865d 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1227,7 +1227,7 @@ mymain(void)
>      DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-device-lun-type-invalid");
>      DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-attaching-partition-nosupport");
>      DO_TEST_CAPS_LATEST("disk-usb-device");
> -    DO_TEST_CAPS_LATEST("disk-usb-device-removable");
> +    DO_TEST_CAPS_LATEST("disk-device-removable");
>      DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-usb-pci");
>      DO_TEST_CAPS_LATEST("disk-scsi");
>      DO_TEST_CAPS_LATEST("disk-scsi-device-auto");
> --
> 2.40.1
>
>
Re: [PATCH] qemu: Support removable for scsi disk
Posted by Michal Prívozník 2 years, 7 months ago 
On 6/5/23 07:07, Han Han wrote:
> Allow //disk/target@removable for scsi disk devices, since QEMU has support
> the removable attribute for scsi-hd device from v0.14.0[1].
> 
> [1]: 419e691f8e: scsi-disk: Allow overriding SCSI INQUIRY removable bit
> 
> Signed-off-by: Han Han <hhan@redhat.com>
> ---
>  docs/formatdomain.rst                              |  2 +-
>  src/conf/domain_validate.c                         |  5 +++--
>  src/qemu/qemu_command.c                            |  6 ++++--
>  ...gs => disk-device-removable.x86_64-latest.args} | 14 +++++++++-----
>  ...ice-removable.xml => disk-device-removable.xml} |  5 +++++
>  tests/qemuxml2argvtest.c                           |  2 +-
>  6 files changed, 23 insertions(+), 11 deletions(-)
>  rename tests/qemuxml2argvdata/{disk-usb-device-removable.x86_64-latest.args => disk-device-removable.x86_64-latest.args} (70%)
>  rename tests/qemuxml2argvdata/{disk-usb-device-removable.xml => disk-device-removable.xml} (80%)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index c3526439bf..676e4b9fd3 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3094,7 +3094,7 @@ paravirtualized driver is specified via the ``disk`` element.
>     CDROM or Floppy disk), the value can be either "open" or "closed", defaults
>     to "closed". NB, the value of ``tray`` could be updated while the domain is
>     running. The optional attribute ``removable`` sets the removable flag for USB
> -   disks, and its value can be either "on" or "off", defaulting to "off".
> +   or SCSI disks, and its value can be either "on" or "off", defaulting to "off".
>     The optional attribute ``rotation_rate`` sets the rotation rate of the
>     storage for disks on a SCSI, IDE, or SATA bus. Values in the range 1025 to
>     65534 are used to indicate rotational media speed in revolutions per minute.
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index 80d6a2ffd9..e7122f6297 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -881,9 +881,10 @@ virDomainDiskDefValidate(const virDomainDef *def,
>      }
>  
>      if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT &&
> -        disk->bus != VIR_DOMAIN_DISK_BUS_USB) {
> +        disk->bus != VIR_DOMAIN_DISK_BUS_USB &&
> +        !(disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)) {

This is a long long line and could be split into two.

>          virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("removable is only valid for usb disks"));
> +                       _("removable is only valid for usb or scsi disks"));
>          return -1;
>      }
>  

The rest looks okay. I'd merge this but it's a feature and we're
currently in the freeze, preparing for upcoming release (i.e. only bug
fixes should be merged). I'll merge this after the release.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
Re: [PATCH] qemu: Support removable for scsi disk
Posted by Michal Prívozník 2 years, 7 months ago 
On 6/29/23 13:03, Michal Prívozník wrote:
> On 6/5/23 07:07, Han Han wrote:
>> Allow //disk/target@removable for scsi disk devices, since QEMU has support
>> the removable attribute for scsi-hd device from v0.14.0[1].
>>
>> [1]: 419e691f8e: scsi-disk: Allow overriding SCSI INQUIRY removable bit
>>
>> Signed-off-by: Han Han <hhan@redhat.com>
>> ---
>>  docs/formatdomain.rst                              |  2 +-
>>  src/conf/domain_validate.c                         |  5 +++--
>>  src/qemu/qemu_command.c                            |  6 ++++--
>>  ...gs => disk-device-removable.x86_64-latest.args} | 14 +++++++++-----
>>  ...ice-removable.xml => disk-device-removable.xml} |  5 +++++
>>  tests/qemuxml2argvtest.c                           |  2 +-
>>  6 files changed, 23 insertions(+), 11 deletions(-)
>>  rename tests/qemuxml2argvdata/{disk-usb-device-removable.x86_64-latest.args => disk-device-removable.x86_64-latest.args} (70%)
>>  rename tests/qemuxml2argvdata/{disk-usb-device-removable.xml => disk-device-removable.xml} (80%)
>>
>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>> index c3526439bf..676e4b9fd3 100644
>> --- a/docs/formatdomain.rst
>> +++ b/docs/formatdomain.rst
>> @@ -3094,7 +3094,7 @@ paravirtualized driver is specified via the ``disk`` element.
>>     CDROM or Floppy disk), the value can be either "open" or "closed", defaults
>>     to "closed". NB, the value of ``tray`` could be updated while the domain is
>>     running. The optional attribute ``removable`` sets the removable flag for USB
>> -   disks, and its value can be either "on" or "off", defaulting to "off".
>> +   or SCSI disks, and its value can be either "on" or "off", defaulting to "off".
>>     The optional attribute ``rotation_rate`` sets the rotation rate of the
>>     storage for disks on a SCSI, IDE, or SATA bus. Values in the range 1025 to
>>     65534 are used to indicate rotational media speed in revolutions per minute.
>> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>> index 80d6a2ffd9..e7122f6297 100644
>> --- a/src/conf/domain_validate.c
>> +++ b/src/conf/domain_validate.c
>> @@ -881,9 +881,10 @@ virDomainDiskDefValidate(const virDomainDef *def,
>>      }
>>  
>>      if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT &&
>> -        disk->bus != VIR_DOMAIN_DISK_BUS_USB) {
>> +        disk->bus != VIR_DOMAIN_DISK_BUS_USB &&
>> +        !(disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)) {
> 
> This is a long long line and could be split into two.
> 
>>          virReportError(VIR_ERR_XML_ERROR, "%s",
>> -                       _("removable is only valid for usb disks"));
>> +                       _("removable is only valid for usb or scsi disks"));
>>          return -1;
>>      }
>>  
> 
> The rest looks okay. I'd merge this but it's a feature and we're
> currently in the freeze, preparing for upcoming release (i.e. only bug
> fixes should be merged). I'll merge this after the release.
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 

Merged now.

Michal
Re: [PATCH] qemu: Support removable for scsi disk
Posted by Han Han 2 years, 7 months ago 
Thanks

On Mon, Jul 10, 2023 at 3:50 PM Michal Prívozník <mprivozn@redhat.com>
wrote:

> On 6/29/23 13:03, Michal Prívozník wrote:
> > On 6/5/23 07:07, Han Han wrote:
> >> Allow //disk/target@removable for scsi disk devices, since QEMU has
> support
> >> the removable attribute for scsi-hd device from v0.14.0[1].
> >>
> >> [1]: 419e691f8e: scsi-disk: Allow overriding SCSI INQUIRY removable bit
> >>
> >> Signed-off-by: Han Han <hhan@redhat.com>
> >> ---
> >>  docs/formatdomain.rst                              |  2 +-
> >>  src/conf/domain_validate.c                         |  5 +++--
> >>  src/qemu/qemu_command.c                            |  6 ++++--
> >>  ...gs => disk-device-removable.x86_64-latest.args} | 14 +++++++++-----
> >>  ...ice-removable.xml => disk-device-removable.xml} |  5 +++++
> >>  tests/qemuxml2argvtest.c                           |  2 +-
> >>  6 files changed, 23 insertions(+), 11 deletions(-)
> >>  rename
> tests/qemuxml2argvdata/{disk-usb-device-removable.x86_64-latest.args =>
> disk-device-removable.x86_64-latest.args} (70%)
> >>  rename tests/qemuxml2argvdata/{disk-usb-device-removable.xml =>
> disk-device-removable.xml} (80%)
> >>
> >> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> >> index c3526439bf..676e4b9fd3 100644
> >> --- a/docs/formatdomain.rst
> >> +++ b/docs/formatdomain.rst
> >> @@ -3094,7 +3094,7 @@ paravirtualized driver is specified via the
> ``disk`` element.
> >>     CDROM or Floppy disk), the value can be either "open" or "closed",
> defaults
> >>     to "closed". NB, the value of ``tray`` could be updated while the
> domain is
> >>     running. The optional attribute ``removable`` sets the removable
> flag for USB
> >> -   disks, and its value can be either "on" or "off", defaulting to
> "off".
> >> +   or SCSI disks, and its value can be either "on" or "off",
> defaulting to "off".
> >>     The optional attribute ``rotation_rate`` sets the rotation rate of
> the
> >>     storage for disks on a SCSI, IDE, or SATA bus. Values in the range
> 1025 to
> >>     65534 are used to indicate rotational media speed in revolutions
> per minute.
> >> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> >> index 80d6a2ffd9..e7122f6297 100644
> >> --- a/src/conf/domain_validate.c
> >> +++ b/src/conf/domain_validate.c
> >> @@ -881,9 +881,10 @@ virDomainDiskDefValidate(const virDomainDef *def,
> >>      }
> >>
> >>      if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT &&
> >> -        disk->bus != VIR_DOMAIN_DISK_BUS_USB) {
> >> +        disk->bus != VIR_DOMAIN_DISK_BUS_USB &&
> >> +        !(disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && disk->device ==
> VIR_DOMAIN_DISK_DEVICE_DISK)) {
> >
> > This is a long long line and could be split into two.
> >
> >>          virReportError(VIR_ERR_XML_ERROR, "%s",
> >> -                       _("removable is only valid for usb disks"));
> >> +                       _("removable is only valid for usb or scsi
> disks"));
> >>          return -1;
> >>      }
> >>
> >
> > The rest looks okay. I'd merge this but it's a feature and we're
> > currently in the freeze, preparing for upcoming release (i.e. only bug
> > fixes should be merged). I'll merge this after the release.
> >
> > Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> >
>
> Merged now.
>
> Michal
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.