On 10/09/2017 04:49 PM, Dawid Zamirski wrote:
> From: Dawid Zamirski <dzamirski@xxxxxxxxx>
>
> The optional values are 'piix3', 'piix4' or 'ich6'. Those will be
> needed to allow setting IDE controller model in VirtualBox driver.
> ---
> docs/schemas/domaincommon.rng | 18 ++++++++++++++++--
> src/conf/domain_conf.c | 9 +++++++++
> src/conf/domain_conf.h | 9 +++++++++
> src/libvirt_private.syms | 2 ++
> 4 files changed, 36 insertions(+), 2 deletions(-)
>
Patches 2 & 3 should be combined and you'll need to provide an xml2xml
example here, modifying tests/qemuxml2xmltest.c in order to add your
test. Search for controller type='scsi' and model= being set in any of
the tests/*/*.xml files. Then generate your test that would have
controller type='ide' ... model='xxx' (just one example is fine).
You may also need to alter virDomainControllerDefValidate to ensure
perhaps some existing assumptions on VIR_DOMAIN_CONTROLLER_TYPE_IDE
aren't lost if ->model is filled in.
I am far from being an expert on IDE controllers and their existing
assumptions, but if you search on VIR_DOMAIN_CONTROLLER_TYPE_IDE
references you will find the existing ones. My assumption has been that
the current model assumption is piix3 - so by adding piix4 and ich6
you'll need to alter code in order to handle any assumptions those
models have; otherwise, once code finds IDE it assumes piix3 and those
assumptions may not work well for piix4 and ich6.
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 4dbda6932..c3f1557f0 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1927,12 +1927,11 @@
> <ref name="address"/>
> </optional>
> <choice>
> - <!-- fdc/ide/sata/ccid have only the common attributes -->
> + <!-- fdc/sata/ccid have only the common attributes -->
> <group>
> <attribute name="type">
> <choice>
> <value>fdc</value>
> - <value>ide</value>
> <value>sata</value>
> <value>ccid</value>
> </choice>
> @@ -1993,6 +1992,21 @@
> </attribute>
> </optional>
> </group>
> + <!-- ide has an optional attribute "model" -->
> + <group>
> + <attribute name="type">
> + <value>ide</value>
> + </attribute>
> + <optional>
> + <attribute name="model">
> + <choice>
> + <value>piix3</value>
> + <value>piix4</value>
> + <value>ich6</value>
> + </choice>
> + </attribute>
> + </optional>
> + </group>
> <!-- pci has an optional attribute "model" -->
> <group>
> <attribute name="type">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 54be9028d..493bf83ff 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -378,6 +378,11 @@ VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST,
> "qemu-xhci",
> "none")
>
> +VIR_ENUM_IMPL(virDomainControllerModelIDE, VIR_DOMAIN_CONTROLLER_MODEL_IDE_LAST,
> + "piix3",
> + "piix4",
> + "ich6")
> +
> VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
> "mount",
> "block",
> @@ -9467,6 +9472,8 @@ virDomainControllerModelTypeFromString(const virDomainControllerDef *def,
> return virDomainControllerModelUSBTypeFromString(model);
> else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI)
> return virDomainControllerModelPCITypeFromString(model);
> + else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE)
> + return virDomainControllerModelIDETypeFromString(model);
>
> return -1;
> }
> @@ -9482,6 +9489,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def,
> return virDomainControllerModelUSBTypeToString(model);
> else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI)
> return virDomainControllerModelPCITypeToString(model);
> + else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE)
> + return virDomainControllerModelIDETypeToString(model);
>
> return NULL;
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a42efcfa6..d7f4c3f1e 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -748,6 +748,14 @@ typedef enum {
> VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST
> } virDomainControllerModelUSB;
>
> +typedef enum {
> + VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX3,
> + VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX4,
> + VIR_DOMAIN_CONTROLLER_MODEL_IDE_ICH6,
> +
> + VIR_DOMAIN_CONTROLLER_MODEL_IDE_LAST
> +} virDomainControllerModelIDE;
> +
> # define IS_USB2_CONTROLLER(ctrl) \
> (((ctrl)->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && \
> ((ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1 || \
> @@ -3231,6 +3239,7 @@ VIR_ENUM_DECL(virDomainControllerModelPCI)
> VIR_ENUM_DECL(virDomainControllerPCIModelName)
> VIR_ENUM_DECL(virDomainControllerModelSCSI)
> VIR_ENUM_DECL(virDomainControllerModelUSB)
> +VIR_ENUM_DECL(virDomainControllerModelIDE)
> VIR_ENUM_DECL(virDomainFS)
> VIR_ENUM_DECL(virDomainFSDriver)
> VIR_ENUM_DECL(virDomainFSAccessMode)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9243c5591..616b14f82 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -231,6 +231,8 @@ virDomainControllerFindUnusedIndex;
> virDomainControllerInsert;
> virDomainControllerInsertPreAlloced;
> virDomainControllerIsPSeriesPHB;
> +virDomainControllerModelIDETypeFromString;
> +virDomainControllerModelIDETypeToString;
> virDomainControllerModelPCITypeToString;
> virDomainControllerModelSCSITypeFromString;
> virDomainControllerModelSCSITypeToString;
>
"Currently" you probably don't need to export the *IDEType{To|From}*
functions since domain_conf is the only consumer; however, seeing how
the *SCSIType{To|From}* has multiple/external consumers makes me wonder
if more consumers are needed for IDE.
In particular, in qemuBuildControllerDevStr and some assumptions in
src/qemu/qemu_domain_address.c related to where controllers live on the
bus for piix3 (FWIW: This was written before the above last two
paragraphs - it's what prompted me to consider the needs).
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
