Re: [Xen-devel] [PATCH 16/18] xen: automatically create XenQdiskDevice-s

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Nov 21, 2018 at 03:12:09PM +0000, Paul Durrant wrote:
> This patch adds a creator function for XenQdiskDevice-s so that they can
> be created automatically when the Xen toolstack instantiates a new
> PV backend. When the XenQdiskDevice is created this way it is also
> necessary to create a drive which matches the configuration that the Xen
> toolstack has written into xenstore. This drive is marked 'auto_del' so
> that it will be removed when the XenQdiskDevice is destroyed. Also, for
> compatibilitye with the legacy 'xen_disk' implementation, an iothread
> is automatically created for the new XenQdiskDevice. This will also be
> removed when he XenQdiskDevice is destroyed.

"the XenQdiskDevice"

[...]
> +    qemu_opt_set(drive_opts, "file.locking", "off", &local_err);

That looks new, compared to the xen_disk.c implementation. Maybe that
should be mention in the commit message.


[..]

> +static void xen_qdisk_device_create(BusState *bus, const char *name,
> +                                    QDict *opts, Error **errp)
> +{
[...]
> +    iothread = iothread_create(vdev, &error_abort);

I would just propagate the error, since iothread could fail for external
reason. No need to crash qemu while a VM is running.

> +
> +    dev = qdev_create(bus, TYPE_XEN_QDISK_DEVICE);
> +
> +    qdev_prop_set_string(dev, "vdev", vdev);
> +
> +    if (XEN_QDISK_DEVICE(dev)->vdev.number != number) {
> +        error_setg(errp, "invalid dev parameter '%s'", vdev);
> +        goto unref;
> +    }
> +
> +    qdev_prop_set_drive(dev, "drive", blk, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        error_prepend(errp, "failed to set 'drive': ");
> +        goto unref;
> +    }
> +
> +    XEN_QDISK_DEVICE(dev)->auto_iothread = iothread;
> +
> +    qdev_init_nofail(dev);

That function shouldn't be use during hotplug. But I'm not sure what
should be done instead, probably object_property_set_bool(..., true
"realized") and check for error.


Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel