Re: [libvirt] [PATCH v2 10/15] vbox: Process element in domain XML
On Fri, 2017-11-03 at 09:51 -0400, John Ferlan wrote:
>
> On 10/24/2017 03:35 PM, Dawid Zamirski wrote:
> >
> > +case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> > +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SCSI_NAME,
> > );
> > +vboxBusType = StorageBus_SCSI;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> > +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SATA_NAME,
> > );
> > +vboxBusType = StorageBus_SATA;
>
>
> I think prior to this patch - there should be a patch that "manages"
> all
> those range check differences, storageBus comparison checks, etc. for
> VIR_DOMAIN_CONTROLLER_TYPE_SATA that end up in future patches.
>
> John
Up until this patch, the series modifies the definexml functionality in
which case input is libvirt XML that is translated to vbox values. The
supported libvirt types are 'validated' by the typed switch...case
statement so it's not possible to run into a vbox value that the driver
does not recognize. The patches that follow are for dumpxml and those
do get updated to handle StorageBus_SAS (the new one introduced in this
series) in patch #12 (see vboxGenerateMediumName change) and in patch
#15 where vboxDumpDisks is changed to use typed switch statement on
storageBus.
Dawid
>
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_USB:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("The vbox driver does not support %s
> > controller type"),
> > + virDomainControllerTypeToString(controller-
> > >type));
> > +return -1;
> > +}
> > +
> > +/* libvirt scsi model => vbox scsi model */
> > +if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
> > +switch ((virDomainControllerModelSCSI) controller->model)
> > {
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
> > +vboxModel = StorageControllerType_LsiLogic;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
> > +vboxModel = StorageControllerType_BusLogic;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
> > +/* in vbox, lsisas has a dedicated SAS bus type with
> > no model */
> > +VBOX_UTF16_FREE(controllerName);
> > +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SAS_NAME,
> > );
> > +vboxBusType = StorageBus_SAS;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
> > +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("The vbox driver does not support %s
> > SCSI "
> > + "controller model"),
> > + virDomainControllerModelSCSITypeToStrin
> > g(controller->model));
> > +goto cleanup;
> > +}
> > +/* libvirt ide model => vbox ide model */
> > +} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE)
> > {
> > +switch ((virDomainControllerModelIDE) controller->model) {
> > +case VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX3:
> > +vboxModel = StorageControllerType_PIIX3;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX4:
> > +vboxModel = StorageControllerType_PIIX4;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_MODEL_IDE_ICH6:
> > +vboxModel = StorageControllerType_ICH6;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_MODEL_IDE_LAST:
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("The vbox driver does not support %s
> > IDE "
> > + "controller model"),
> > + virDomainControllerModelIDETypeToStri
> > ng(controller->model));
> > +goto cleanup;
> > +}
> > +}
> > +
> > +VBOX_UTF16_TO_UTF8(controllerName, );
> > +VIR_DEBUG("Adding VBOX storage controller (name: %s, busType:
> > %d)",
> > + debugName, vboxBusType);
> > +
> > +rc = gVBoxAPI.UIMachine.AddStorageController(machine,
> > controllerName,
> > + vboxBusType,
> > );
> > +
> > +if (NS_FAILED(rc)) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("Failed to add storage controller "
> > + "(name: %s, busType: %d), rc=%08x"),
> > +
Re: [libvirt] [PATCH v2 10/15] vbox: Process element in domain XML
On Fri, 2017-11-03 at 09:51 -0400, John Ferlan wrote:
>
> On 10/24/2017 03:35 PM, Dawid Zamirski wrote:
> > This patch enables the VBOX driver to process the
> > element
> > in domain XML through which one can now customize the controller
> > model.
> >
> > Since VirtualBox has two distinct SAS and SCSI they do not "map"
> > directly to libvirt XML, he VBOX driver uses > model="lsisas1068" /> to create SAS controller in VBOX VM.
> > Additionally
> > once can set model on the IDE controller to be one of "piix3",
> > "piix4"
> > or "ich6".
> > ---
> > src/vbox/vbox_common.c | 214
> > ++---
> > src/vbox/vbox_common.h | 8 ++
> > 2 files changed, 176 insertions(+), 46 deletions(-)
> >
>
> So beyond patch 3 which I know you have to repost anyway - starting
> with
> this patch and going forward, I figure you have to repost anyway as
> long
> as I get the question in patch 5 answered of course...
>
> This patch left me with a concern. Up to this point vboxAttachDrives
> would add at least one controller for each type supported regardless
> of
> whether the VM used it.
>
> After this patch only those controllers defined in the VM XML will be
> added. Which would seem to be fine, except for the case of hotplug
> which
> I wasn't clear whether vbox support or not.
>
> Let's say the VM is defined with and IDE controller, if someone tried
> to
> add a SCSI disk after startup, then there'd be no SCSI controller
> already present.
>
I've just checked this with VirutalBox GUI and the only thing that can
be changed on live VM is removable media (CD/DVD). All options to add
disk devices to controller are disabled when VM is running. Neither
quick web search returned anything useful regarding hot-add disk
support on VBOX.
Having said that, the original behavior was, IIRC, the reason why I
actually started working on this series. According to our tech support
they have ran into a couple of cases where the OS would BSOD (some
legacy Windows OS e.g. 2000 or XP) when there was a controller (without
a disk) attached to VM that the OS did not handle well. It sounded very
strange to me but I've been told it was not a one-off case though I
personally haven't reproduced it. Unfortunately, I won't be able to
provide more details as we no longer use VBox internally (migrated to
KVM) so those potential test cases are no longer around.
>
> > diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> > index 2bd891efb..9d45e4a76 100644
> > --- a/src/vbox/vbox_common.c
> > +++ b/src/vbox/vbox_common.c
> > @@ -406,6 +406,160 @@ static char *vboxGenerateMediumName(PRUint32
> > storageBus,
> > return name;
> > }
> >
> > +
> > +static int
> > +vboxSetStorageController(virDomainControllerDefPtr controller,
> > + vboxDriverPtr data,
> > + IMachine *machine)
> > +{
> > +PRUnichar *controllerName = NULL;
> > +PRInt32 vboxModel = StorageControllerType_Null;
> > +PRInt32 vboxBusType = StorageBus_Null;
> > +IStorageController *vboxController = NULL;
> > +nsresult rc = 0;
> > +char *debugName = NULL;
> > +int ret = -1;
> > +
> > +/* libvirt controller type => vbox bus type */
> > +switch ((virDomainControllerType) controller->type) {
> > +case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
> > +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_FLOPPY_NAME,
> > );
> > +vboxBusType = StorageBus_Floppy;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> > +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_IDE_NAME,
> > );
> > +vboxBusType = StorageBus_IDE;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> > +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SCSI_NAME,
> > );
> > +vboxBusType = StorageBus_SCSI;
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> > +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SATA_NAME,
> > );
> > +vboxBusType = StorageBus_SATA;
>
>
> I think prior to this patch - there should be a patch that "manages"
> all
> those range check differences, storageBus comparison checks, etc. for
> VIR_DOMAIN_CONTROLLER_TYPE_SATA that end up in future patches.
>
> John
>
> > +
> > +break;
> > +case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_USB:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> > +case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("The vbox driver does not support %s
> > controller type"),
> > + virDomainControllerTypeToString(controller-
> > >type));
> > +return -1;
> > +}
> > +
> > +/* libvirt scsi model => vbox scsi model */
> > +if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
> > +switch ((virDomainControllerModelSCSI) controller->model)
> > {
> > +
Re: [libvirt] [PATCH v2 10/15] vbox: Process element in domain XML
On 10/24/2017 03:35 PM, Dawid Zamirski wrote:
> This patch enables the VBOX driver to process the element
> in domain XML through which one can now customize the controller model.
>
> Since VirtualBox has two distinct SAS and SCSI they do not "map"
> directly to libvirt XML, he VBOX driver uses model="lsisas1068" /> to create SAS controller in VBOX VM. Additionally
> once can set model on the IDE controller to be one of "piix3", "piix4"
> or "ich6".
> ---
> src/vbox/vbox_common.c | 214
> ++---
> src/vbox/vbox_common.h | 8 ++
> 2 files changed, 176 insertions(+), 46 deletions(-)
>
So beyond patch 3 which I know you have to repost anyway - starting with
this patch and going forward, I figure you have to repost anyway as long
as I get the question in patch 5 answered of course...
This patch left me with a concern. Up to this point vboxAttachDrives
would add at least one controller for each type supported regardless of
whether the VM used it.
After this patch only those controllers defined in the VM XML will be
added. Which would seem to be fine, except for the case of hotplug which
I wasn't clear whether vbox support or not.
Let's say the VM is defined with and IDE controller, if someone tried to
add a SCSI disk after startup, then there'd be no SCSI controller
already present.
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index 2bd891efb..9d45e4a76 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -406,6 +406,160 @@ static char *vboxGenerateMediumName(PRUint32 storageBus,
> return name;
> }
>
> +
> +static int
> +vboxSetStorageController(virDomainControllerDefPtr controller,
> + vboxDriverPtr data,
> + IMachine *machine)
> +{
> +PRUnichar *controllerName = NULL;
> +PRInt32 vboxModel = StorageControllerType_Null;
> +PRInt32 vboxBusType = StorageBus_Null;
> +IStorageController *vboxController = NULL;
> +nsresult rc = 0;
> +char *debugName = NULL;
> +int ret = -1;
> +
> +/* libvirt controller type => vbox bus type */
> +switch ((virDomainControllerType) controller->type) {
> +case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
> +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_FLOPPY_NAME, );
> +vboxBusType = StorageBus_Floppy;
> +
> +break;
> +case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_IDE_NAME, );
> +vboxBusType = StorageBus_IDE;
> +
> +break;
> +case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SCSI_NAME, );
> +vboxBusType = StorageBus_SCSI;
> +
> +break;
> +case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SATA_NAME, );
> +vboxBusType = StorageBus_SATA;
I think prior to this patch - there should be a patch that "manages" all
those range check differences, storageBus comparison checks, etc. for
VIR_DOMAIN_CONTROLLER_TYPE_SATA that end up in future patches.
John
> +
> +break;
> +case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
> +case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
> +case VIR_DOMAIN_CONTROLLER_TYPE_USB:
> +case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> +case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("The vbox driver does not support %s controller
> type"),
> + virDomainControllerTypeToString(controller->type));
> +return -1;
> +}
> +
> +/* libvirt scsi model => vbox scsi model */
> +if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
> +switch ((virDomainControllerModelSCSI) controller->model) {
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
> +vboxModel = StorageControllerType_LsiLogic;
> +
> +break;
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
> +vboxModel = StorageControllerType_BusLogic;
> +
> +break;
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
> +/* in vbox, lsisas has a dedicated SAS bus type with no model */
> +VBOX_UTF16_FREE(controllerName);
> +VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SAS_NAME, );
> +vboxBusType = StorageBus_SAS;
> +
> +break;
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
> +case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("The vbox driver does not support %s SCSI "
> + "controller model"),
> +
>
[libvirt] [PATCH v2 10/15] vbox: Process element in domain XML
This patch enables the VBOX driver to process the element
in domain XML through which one can now customize the controller model.
Since VirtualBox has two distinct SAS and SCSI they do not "map"
directly to libvirt XML, he VBOX driver uses to create SAS controller in VBOX VM. Additionally
once can set model on the IDE controller to be one of "piix3", "piix4"
or "ich6".
---
src/vbox/vbox_common.c | 214 ++---
src/vbox/vbox_common.h | 8 ++
2 files changed, 176 insertions(+), 46 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 2bd891efb..9d45e4a76 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -406,6 +406,160 @@ static char *vboxGenerateMediumName(PRUint32 storageBus,
return name;
}
+
+static int
+vboxSetStorageController(virDomainControllerDefPtr controller,
+ vboxDriverPtr data,
+ IMachine *machine)
+{
+PRUnichar *controllerName = NULL;
+PRInt32 vboxModel = StorageControllerType_Null;
+PRInt32 vboxBusType = StorageBus_Null;
+IStorageController *vboxController = NULL;
+nsresult rc = 0;
+char *debugName = NULL;
+int ret = -1;
+
+/* libvirt controller type => vbox bus type */
+switch ((virDomainControllerType) controller->type) {
+case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
+VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_FLOPPY_NAME, );
+vboxBusType = StorageBus_Floppy;
+
+break;
+case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
+VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_IDE_NAME, );
+vboxBusType = StorageBus_IDE;
+
+break;
+case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
+VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SCSI_NAME, );
+vboxBusType = StorageBus_SCSI;
+
+break;
+case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SATA_NAME, );
+vboxBusType = StorageBus_SATA;
+
+break;
+case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
+case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
+case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("The vbox driver does not support %s controller
type"),
+ virDomainControllerTypeToString(controller->type));
+return -1;
+}
+
+/* libvirt scsi model => vbox scsi model */
+if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
+switch ((virDomainControllerModelSCSI) controller->model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
+vboxModel = StorageControllerType_LsiLogic;
+
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
+vboxModel = StorageControllerType_BusLogic;
+
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
+/* in vbox, lsisas has a dedicated SAS bus type with no model */
+VBOX_UTF16_FREE(controllerName);
+VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SAS_NAME, );
+vboxBusType = StorageBus_SAS;
+
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("The vbox driver does not support %s SCSI "
+ "controller model"),
+
virDomainControllerModelSCSITypeToString(controller->model));
+goto cleanup;
+}
+/* libvirt ide model => vbox ide model */
+} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) {
+switch ((virDomainControllerModelIDE) controller->model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX3:
+vboxModel = StorageControllerType_PIIX3;
+
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX4:
+vboxModel = StorageControllerType_PIIX4;
+
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_IDE_ICH6:
+vboxModel = StorageControllerType_ICH6;
+
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_IDE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("The vbox driver does not support %s IDE "
+ "controller model"),
+
virDomainControllerModelIDETypeToString(controller->model));
+goto cleanup;
+}
+}
+
+VBOX_UTF16_TO_UTF8(controllerName, );
+VIR_DEBUG("Adding VBOX storage controller (name: %s, busType: %d)",
+ debugName, vboxBusType);
+
+rc =
