[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 12/13] qemu: only add channel arg to scsi-disk if qemu binary supports it




On 05/05/2015 02:03 PM, Laine Stump wrote:
> libvirt enforces bus (channel to qemu) == 0 for those qemu binaries
> that don't support the channel argument to scsi disk devices, but
> still adds "channel=0" in those cases. Apparently nobody with a qemu
> old enough to not support channel ever uses these devices, because
> they otherwise should get an error.
> 
> This patch only adds channel when the scsi-disk.channel capability is
> set for the qemu binary. Most of the change in the patch is updating
> patches to either 1) remove channel=0 from the .args file of the test,
> or 2) reposition channel=0 to precede the bus arg (because this makes
> the code cleaner/simpler) and change the DO_TEST() invocation for the
> test to add QEMU_CAPS_SCSI_DISK_CHANNEL.
> 
> I'm uncommitted about whether or not it is worthwhile to push this
> patch. On one hand, I'm fairly certain this is what is correct; on the
> other hand nobody has ever complained about it, and at this point
> almost everyone is using new enough qemu that it supports channel.
> ---
>  src/qemu/qemu_command.c                            |  7 +++---
>  .../qemuxml2argv-disk-drive-network-iscsi-lun.args |  2 +-
>  .../qemuxml2argv-disk-scsi-disk-split.args         |  8 +++----
>  .../qemuxml2argv-disk-scsi-disk-vpd.args           |  2 +-
>  .../qemuxml2argv-disk-scsi-disk-wwn.args           |  4 ++--
>  .../qemuxml2argv-disk-scsi-lun-passthrough.args    |  4 ++--
>  .../qemuxml2argv-disk-scsi-megasas.args            |  2 +-
>  .../qemuxml2argv-disk-scsi-virtio-scsi.args        |  2 +-
>  .../qemuxml2argv-disk-scsi-vscsi.args              |  2 +-
>  .../qemuxml2argv-disk-virtio-scsi-ccw.args         |  2 +-
>  .../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args |  2 +-
>  .../qemuxml2argv-disk-virtio-scsi-max_sectors.args |  2 +-
>  .../qemuxml2argv-disk-virtio-scsi-num_queues.args  |  2 +-
>  .../qemuxml2argv-pseries-vio-user-assigned.args    |  2 +-
>  .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args |  2 +-
>  tests/qemuxml2argvtest.c                           | 25 +++++++++++++---------
>  16 files changed, 38 insertions(+), 32 deletions(-)
> 

I'm in agreement with you vis-a-vis whether it's worthwhile. Nothing's
broken, so the axiom may apply. However, we don't know if it's broken
"under the covers" since we don't know the affect of adding channel=# to
a qemu that doesn't support it since we don't seem to have one of those
today.

OTOH since you added the QEMU_CAPS_SCSI_DISK_CHANNEL bit in and that
alone didn't break anything, then it would appear this change should be
OK.  Other than changing the order/placement of channel (which seems to
be a non-issue), it seems these changes are fine.

Should I assume you've started guests with all these changes? And
perhaps restart libvirtd to make sure noone disappears?

quasi-ACK, your call depending on your comfort level.

John



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]