[libvirt] qemu: hotplug virtio_scsi over lsilogic when sicsi controller is not present?

Hi, We hit some problems when we attached some lun devices in our vm, turns out that libvirt created lsilogic scsi controller automatically for these scsi devices, however these device works well under virtio_scsi controller. the current code logic is check lsilogic first, if qemu could not support it, then check virtio_scsi, however is it better to check virtio_scsi earlier? since it is supported better in qemu level? I am wondering which solution is better? 1. simple switch sequence and check virtio_scsi first 2. add extra option for "virsh attach-disk" to choose specific controller type Thanks, Liang ==================================== Code part is like below qemu_hotplug.c qemuDomainFindOrCreateSCSIDiskController /* No SCSI controller present, for backward compatibility we * now hotplug a controller */ if (VIR_ALLOC(cont) < 0) return NULL; cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; cont->idx = controller; cont->model = -1; VIR_INFO("No SCSI controller present, hotplugging one"); if (qemuDomainAttachControllerDevice(driver, qemu_domain_address.c qemuDomainSetSCSIControllerModel if (qemuDomainIsPSeries(def)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to determine model for scsi controller")); return -1;

On Tue, Jul 11, 2017 at 03:47:55PM -0400, Liang Yan wrote:
Hi,
We hit some problems when we attached some lun devices in our vm, turns out that libvirt created lsilogic scsi controller automatically for these scsi devices, however these device works well under virtio_scsi controller.
the current code logic is check lsilogic first, if qemu could not support it, then check virtio_scsi, however is it better to check virtio_scsi earlier? since it is supported better in qemu level?
It is not in older QEMUs which we need to stay compatible with.
I am wondering which solution is better? 1. simple switch sequence and check virtio_scsi first
We can't do that. We have to keep parsing older XMLs (that did not have the model in them) the same way as we were before so that we keep stable guest ABI.
2. add extra option for "virsh attach-disk" to choose specific controller type
Or you can first attach the controller and then attach-device with the xml that specifies that particular controller. attach-disk is a syntactic sugar for attach-device IIRC.
Thanks, Liang
==================================== Code part is like below
qemu_hotplug.c qemuDomainFindOrCreateSCSIDiskController
/* No SCSI controller present, for backward compatibility we * now hotplug a controller */ if (VIR_ALLOC(cont) < 0) return NULL; cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; cont->idx = controller; cont->model = -1;
VIR_INFO("No SCSI controller present, hotplugging one"); if (qemuDomainAttachControllerDevice(driver,
qemu_domain_address.c qemuDomainSetSCSIControllerModel
if (qemuDomainIsPSeries(def)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to determine model for scsi controller")); return -1;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 7/14/17 3:56 AM, Martin Kletzander wrote:
On Tue, Jul 11, 2017 at 03:47:55PM -0400, Liang Yan wrote:
Hi,
We hit some problems when we attached some lun devices in our vm, turns out that libvirt created lsilogic scsi controller automatically for these scsi devices, however these device works well under virtio_scsi controller.
the current code logic is check lsilogic first, if qemu could not support it, then check virtio_scsi, however is it better to check virtio_scsi earlier? since it is supported better in qemu level?
It is not in older QEMUs which we need to stay compatible with.
Yes, Peter said similar things that kernel may not include newer driver.
I am wondering which solution is better? 1. simple switch sequence and check virtio_scsi first
We can't do that. We have to keep parsing older XMLs (that did not have the model in them) the same way as we were before so that we keep stable guest ABI.
2. add extra option for "virsh attach-disk" to choose specific controller type
Or you can first attach the controller and then attach-device with the xml that specifies that particular controller. attach-disk is a syntactic sugar for attach-device IIRC.
Yes, this is our current workaround. Some users are just wondering if could finish this process by command line(attach-disk) in one step. I checked the options of attach-disk that "-targetbus" is a close one but focus on high level bus controller. Anyway, thanks for the response, I think what I need to do now is update our document to make it more clearly. Please let me know if you have different decision in the future. Thanks, Liang
Thanks, Liang
==================================== Code part is like below
qemu_hotplug.c qemuDomainFindOrCreateSCSIDiskController
/* No SCSI controller present, for backward compatibility we * now hotplug a controller */ if (VIR_ALLOC(cont) < 0) return NULL; cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; cont->idx = controller; cont->model = -1;
VIR_INFO("No SCSI controller present, hotplugging one"); if (qemuDomainAttachControllerDevice(driver,
qemu_domain_address.c qemuDomainSetSCSIControllerModel
if (qemuDomainIsPSeries(def)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to determine model for scsi controller")); return -1;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Jul 14, 2017 at 08:52:35 -0400, Liang Yan wrote:
On 7/14/17 3:56 AM, Martin Kletzander wrote:
On Tue, Jul 11, 2017 at 03:47:55PM -0400, Liang Yan wrote:
Hi,
We hit some problems when we attached some lun devices in our vm, turns out that libvirt created lsilogic scsi controller automatically for these scsi devices, however these device works well under virtio_scsi controller.
the current code logic is check lsilogic first, if qemu could not support it, then check virtio_scsi, however is it better to check virtio_scsi earlier? since it is supported better in qemu level?
It is not in older QEMUs which we need to stay compatible with.
Yes, Peter said similar things that kernel may not include newer driver.
I am wondering which solution is better? 1. simple switch sequence and check virtio_scsi first
We can't do that. We have to keep parsing older XMLs (that did not have the model in them) the same way as we were before so that we keep stable guest ABI.
2. add extra option for "virsh attach-disk" to choose specific controller type
Or you can first attach the controller and then attach-device with the xml that specifies that particular controller. attach-disk is a syntactic sugar for attach-device IIRC.
Yes, this is our current workaround. Some users are just wondering if could finish this process by command line(attach-disk) in one step. I checked the options of attach-disk that "-targetbus" is a close one but focus on high level bus controller.
I don't think that will be easy to implement. Adding a new controller is a rather complex step which can fail and having a two step operation crammed into a single API is always painful in error cases, since you can end up in a partially modified state which can't be rolled back. The current state was to make it just work, but as you can see, this can't be guessed correctly. The problem also is that one parameter may be enough to get the controller type for now, but in the future it may not be enough as it is not enough just to pick the default one now. I think it would be much more usefull to add a virsh convenience command to attach controllers, so that users can do this in a simple way rather than trying to do it in a single API call.

On 7/14/17 10:33 AM, Peter Krempa wrote:
On Fri, Jul 14, 2017 at 08:52:35 -0400, Liang Yan wrote:
On 7/14/17 3:56 AM, Martin Kletzander wrote:
On Tue, Jul 11, 2017 at 03:47:55PM -0400, Liang Yan wrote:
Hi,
We hit some problems when we attached some lun devices in our vm, turns out that libvirt created lsilogic scsi controller automatically for these scsi devices, however these device works well under virtio_scsi controller.
the current code logic is check lsilogic first, if qemu could not support it, then check virtio_scsi, however is it better to check virtio_scsi earlier? since it is supported better in qemu level?
It is not in older QEMUs which we need to stay compatible with.
Yes, Peter said similar things that kernel may not include newer driver.
I am wondering which solution is better? 1. simple switch sequence and check virtio_scsi first
We can't do that. We have to keep parsing older XMLs (that did not have the model in them) the same way as we were before so that we keep stable guest ABI.
2. add extra option for "virsh attach-disk" to choose specific controller type
Or you can first attach the controller and then attach-device with the xml that specifies that particular controller. attach-disk is a syntactic sugar for attach-device IIRC.
Yes, this is our current workaround. Some users are just wondering if could finish this process by command line(attach-disk) in one step. I checked the options of attach-disk that "-targetbus" is a close one but focus on high level bus controller. I don't think that will be easy to implement. Adding a new controller is a rather complex step which can fail and having a two step operation crammed into a single API is always painful in error cases, since you can end up in a partially modified state which can't be rolled back.
The current state was to make it just work, but as you can see, this can't be guessed correctly.
The problem also is that one parameter may be enough to get the controller type for now, but in the future it may not be enough as it is not enough just to pick the default one now. Yeah, that is my initial plan. it would be easy to provide some choices out of default controllers, but would be ugly if users wants more from here. I think it would be much more usefull to add a virsh convenience command This is a good idea. If customer wants to specify a controller, it won't hurt too much to run another virsh command. Also it avoids merging two big functions in one API. to attach controllers, so that users can do this in a simple way rather than trying to do it in a single API call.
participants (3)
-
Liang Yan
-
Martin Kletzander
-
Peter Krempa