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%)
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
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 > >
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
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
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 > >
© 2016 - 2024 Red Hat, Inc.