On 05/03/2013 02:07 PM, Osier Yang wrote:
> Since it's generic enough to be used by other types in future, I
> put it in <hostdev> as sub-element, though now it's only used by
> scsi host device.
> ---
> docs/formatdomain.html.in | 4 +++
> docs/schemas/domaincommon.rng | 5 +++
> src/conf/domain_conf.c | 6 ++++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 4 +++
> .../qemuxml2argv-hostdev-scsi-readonly.args | 9 ++++++
> .../qemuxml2argv-hostdev-scsi-readonly.xml | 36 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 4 +++
> tests/qemuxml2xmltest.c | 1 +
> 9 files changed, 70 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 488673f..9cd79e5 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2406,6 +2406,10 @@
> could be changed in the future with no impact to domains that
> don't specify anything.
> </dd>
> + <dt><code>readonly</code></dt>
> + <dd>Indicates that the device is readonly, only supported by SCSI host
> + device now. <span class="since">Since 1.0.6</span>
> + </dd>
I just realized your example in 1/25 has the 'readonly' element listed
in the new "<pre> ... </pre> section, but it's not described. I think
you probably need to split that line out of there and then put it into
this patch.
Does anything need to be said here that the underlying QEMU needs to
support the readonly too? That is some version or capability? Since
there is a check later on to actually pass/add readonly to the command
line, it'd surely raise a question some day if someone defined it as
readonly, but then come to find out the domain didn't honor that...
> </dl>
>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index b8d0d60..4fdacab 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3075,6 +3075,11 @@
> <optional>
> <ref name="address"/>
> </optional>
> + <optional>
> + <element name="readonly">
> + <empty/>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9e6b65b..31a8c46 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8724,6 +8724,9 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
> _("SCSI host devices must have address specified"));
> goto error;
> }
> +
> + if (virXPathBoolean("boolean(./readonly)", ctxt))
> + def->readonly = true;
> break;
> }
> }
> @@ -15342,6 +15345,9 @@ virDomainHostdevDefFormat(virBufferPtr buf,
> return -1;
> break;
> }
> +
> + if (def->readonly)
> + virBufferAddLit(buf, "<readonly/>\n");
> virBufferAdjustIndent(buf, -6);
>
> if (virDomainDeviceInfoFormat(buf, def->info,
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 1efae69..5471cd3 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -461,6 +461,7 @@ struct _virDomainHostdevDef {
> int startupPolicy; /* enum virDomainStartupPolicy */
> bool managed;
> bool missing;
> + bool readonly;
> union {
> virDomainHostdevSubsys subsys;
> virDomainHostdevCaps caps;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index df896aa..2b141d1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4702,6 +4702,10 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
> virDomainDeviceAddressTypeToString(dev->info->type),
> dev->info->alias);
>
> + if (dev->readonly &&
> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY))
> + virBufferAsprintf(&buf, ",readonly=on");
> +
See comment above. If this is defined as readonly, but the underlying
system doesn't support it, then should we fail? It'd be bad news to
have a domain expect something readonly, but still load and potentially
write on something it wasn't supposed to.
If there's a precedent already that allows a defined readonly element to
be used read/write, then I guess it's a weak ACK, but otherwise, I think
it might be best to fail.
John
> if (virBufferError(&buf)) {
> virReportOOMError();
> goto error;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args
> new file mode 100644
> index 0000000..ea2f7af
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.args
> @@ -0,0 +1,9 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
> +pc -m 214 -smp 1 -nographic -nodefaults -monitor \
> +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
> +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb \
> +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-drive file=/dev/sg0,if=none,id=drive-hostdev-scsi_host0-0-0-0,readonly=on \
> +-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,drive=drive-hostdev-scsi_host0-0-0-0,id=hostdev-scsi_host0-0-0-0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.xml
> new file mode 100644
> index 0000000..359bb95
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-readonly.xml
> @@ -0,0 +1,36 @@
> +<domain type='qemu'>
> + <name>QEMUGuest2</name>
> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219100</memory>
> + <currentMemory unit='KiB'>219100</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <disk type='block' device='disk'>
> + <source dev='/dev/HostVG/QEMUGuest2'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
> + <controller type='scsi' index='0' model='virtio-scsi'/>
> + <controller type='usb' index='0'/>
> + <controller type='ide' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <hostdev mode='subsystem' type='scsi' managed='yes'>
> + <source>
> + <adapter name='scsi_host0'/>
> + <address bus='0' target='0' unit='0'/>
> + </source>
> + <readonly/>
> + <address type='drive' controller='0' bus='0' target='4' unit='8'/>
> + </hostdev>
> + <memballoon model='virtio'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index abe04b3..4d7f5e2 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -982,6 +982,10 @@ mymain(void)
> QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
> QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI,
> QEMU_CAPS_DEVICE_SCSI_GENERIC);
> + DO_TEST("hostdev-scsi-readonly", QEMU_CAPS_DRIVE,
> + QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
> + QEMU_CAPS_DRIVE_READONLY, QEMU_CAPS_VIRTIO_SCSI,
> + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_SCSI_GENERIC);
>
> virObjectUnref(driver.config);
> virObjectUnref(driver.caps);
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 08c3eeb..492ac60 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -287,6 +287,7 @@ mymain(void)
>
> DO_TEST("hostdev-scsi-lsi");
> DO_TEST("hostdev-scsi-virtio-scsi");
> + DO_TEST("hostdev-scsi-readonly");
>
> virObjectUnref(driver.caps);
> virObjectUnref(driver.xmlopt);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list