On 07/22/2015 03:11 PM, John Ferlan wrote:
On 07/17/2015 02:43 PM, Laine Stump wrote:
> There are some non-0 default values in virDomainControllerDef (and
> will soon be more) that are easier to not forget if the remembering is
> done by a single initializer function (rather than inline code after
> allocating the obejct with generic VIR_ALLOC().
> ---
> new in V2
>
> src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5a9a88d..8dd4bf0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1527,6 +1527,37 @@ virDomainDiskSetFormat(virDomainDiskDefPtr def, int format)
> }
>
>
> +static virDomainControllerDefPtr
> +virDomainControllerDefNew(virDomainControllerType type)
> +{
> + virDomainControllerDefPtr def;
> +
> + if (VIR_ALLOC(def) < 0)
> + return NULL;
> +
> + def->type = type;
> +
> + /* initialize anything that has a non-0 default */
> + switch ((virDomainControllerType) def->type) {
> + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
> + def->opts.vioserial.ports = -1;
> + def->opts.vioserial.vectors = -1;
> + break;
> + case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> + case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> + case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
> + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> + case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> + case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
> + case VIR_DOMAIN_CONTROLLER_TYPE_USB:
> + case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
> + break;
> + }
> +
> + return def;
> +}
> +
> +
> void virDomainControllerDefFree(virDomainControllerDefPtr def)
> {
> if (!def)
> @@ -7597,9 +7628,10 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> xmlXPathContextPtr ctxt,
> unsigned int flags)
> {
> - virDomainControllerDefPtr def;
> + virDomainControllerDefPtr def = NULL;
> + int type = 0;
Should we make this?
type = VIR_DOMAIN_CONTROLLER_TYPE_IDE;
Not that it perhaps matters too much, but does perhaps point to
where/why an IDE controller got created if "type='xxx'"
I think it makes more sense to have a separate patch that logs an error
if no type is given - the RNG requires it.