On 01/05/2012 04:59 PM, Laine Stump wrote:
On 01/05/2012 03:44 AM, Paolo Bonzini wrote:
Summary: two nits, one in the docs and one at the end of this email.
[Osier, I'm CCing you because there is some food for thought for SCSI].
On 01/05/2012 05:17 AM, Laine Stump wrote:
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 18b7e22..dcdf91f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1001,8 +1001,16 @@
"block", "dir", or "network"
and refers to the underlying source for the disk. The optional
<code>device</code> attribute indicates how the disk is to be exposed
- to the guest OS. Possible values for this attribute are "floppy",
"disk"
- and "cdrom", defaulting to "disk". The
+ to the guest OS. Possible values for this attribute are
+ "floppy", "disk", "cdrom", and "lun", defaulting to
+ "disk". "lun" (<span class="since">since 0.9.10</span>) is only
+ valid when type is "block" and the target element's "bus"
+ attribute is "virtio", and behaves identically to "disk",
+ except that generic SCSI commands from the guest are accepted
What about "are forwarded to the disk"?
Okay, I'm adding that to the end of the sentence.
This is also true in the SCSI
case (for SCSI, "block" will emulate commands rather than fail them; but
for "lun" the behavior is identical).
We can add that to the docs when virtio-blk-scsi support is added.
Actually, there's already support for SCSI devices with device='disk' or
device='cdrom', just not for the virtio implementation of SCSI. So it
could be added now.
+ - also note that device='lun' will only be recognized for
+ actual raw devices, never for individual partitions or LVM
+ partitions (in those cases, the kernel will reject the generic
+ SCSI commands, making it identical to device='disk'). The
Perhaps add a note that QEMU may fail to start? Again, this is not the
case for virtio but it will be for "scsi".
I'd rather add that information when it actually is true (i.e. when
virtio-blk-scsi support is put in).
More precisely, when support for device='lun' is added for SCSI disks
(and again, it will work also for other controllers, not just
virtio-scsi). So that's fine to leave it as is.
Suboptimal, because a type='lun' disk could well be a CD. :/ But
short of supporting bootindex directly in the<disk> and<network>
items, we cannot do much about it though.
So this is a problem, but not fixable now, correct?
Yes.
Side note: this fails if you're trying to hot-plug a SCSI CD drive.
Probably this error condition in qemuDomainChangeEjectableMedia:
[snip]
should instead fall back to device hot-plug for VIR_DOMAIN_DISK_BUS_SCSI.
Just to verify - this is another thing that you've noticed in the code,
but is orthogonal to this patch, correct?
Yes.
Truthfully, I didn't totally understand what that code was doing, but it
seemed appropriate to at least duplicate the behavior of DEVICE_DISK :-)
Looking at it some more, I *think* what is necessary, is to always
return false if there is a disk that is DEVICE_LUN, since really the
only thing that can be snapshotted is a qcow2 disk, and DEVICE_LUN disks
by definition are never qcow2. So does it seem reasonable to change this
to:
- if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK&&
- (!vm->def->disks[i]->driverType ||
- STRNEQ(vm->def->disks[i]->driverType, "qcow2"))) {
+ if ((vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_LUN) ||
+ (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK&&
+ STRNEQ_NULLABLE(vm->def->disks[i]->driverType, "qcow2"))) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
Makes sense.
Paolo
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list