Thanks
On Mon, Jul 10, 2023 at 3:50 PM Michal Prívozník <mpriv...@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 <h...@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 <mpriv...@redhat.com>
> >
>
> Merged now.
>
> Michal
>
>
