On Wed, Sep 11, 2019 at 03:17:42PM +0200, Michal Privoznik wrote:
> In domain_conf.c we have virDomainSCSIDriveAddressIsUsed()
> function which returns true or false if given drive address is
> already in use for given domain config or not. However, it also
> takes a shortcut and returns true (meaning address in use) if the
> unit number equals 7. This is because for some controllers this
> is reserved address. The limitation comes mostly from vmware and
> applies to lsilogic, buslogic, spapr-vscsi and vmpvscsi models.
> On the other hand, we were not checking for the maximum unit
> number (aka LUN number) which is also relevant and differs from
> model to model.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 53 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 69c486f382..3e7603891f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4813,11 +4813,54 @@ bool
> virDomainSCSIDriveAddressIsUsed(const virDomainDef *def,
> const virDomainDeviceDriveAddress *addr)
> {
> - /* In current implementation, the maximum unit number of a controller
> - * is either 16 or 7 (narrow SCSI bus), and if the maximum unit number
> - * is 16, the controller itself is on unit 7 */
> - if (addr->unit == 7)
> - return true;
> + const virDomainControllerDef *cont;
> +
> + cont = virDomainDeviceFindSCSIController(def, addr);
> + if (cont) {
> + int max = -1;
> + int reserved = -1;
> +
> + /* Different controllers have different limits. These limits here are
> + * taken from QEMU source code, but nevertheless they should apply to
> + * other hypervisors too. */
> + switch ((virDomainControllerModelSCSI) cont->model) {
> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL:
> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_NON_TRANSITIONAL:
> + max = 16383;
> + break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
> + max = 31;
> + reserved = 7;
> + break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
> + max = 1;
> + break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
> + max = 255;
> + break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
> + max = 0;
Surely this ^^^ means....
> + if (max != -1 && addr->unit >= max)
> + return true;
...this is always true and so we'll be unable to attach
a drive to any LUN at all.
> + if (reserved != -1 && addr->unit == reserved)
> + return true;
> + }
>
> if (virDomainDriveAddressIsUsedByDisk(def, VIR_DOMAIN_DISK_BUS_SCSI,
> addr) ||
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
