Re: [Qemu-devel] [PATCH 14/18] xen: add implementations of xen-qdisk connect and disconnect functions...

[Anthony PERARD]

On Wed, Nov 21, 2018 at 03:12:07PM +0000, Paul Durrant wrote:
> diff --git a/hw/block/xen-qdisk.c b/hw/block/xen-qdisk.c
> index 35f7b70480..8c88393832 100644
> --- a/hw/block/xen-qdisk.c
> +++ b/hw/block/xen-qdisk.c
>  static void xen_qdisk_connect(XenQdiskDevice *qdiskdev, Error **errp)
>  {
>      XenQdiskVdev *vdev = &qdiskdev->vdev;
> +    XenDevice *xendev = XEN_DEVICE(qdiskdev);
> +    unsigned int order, nr_ring_ref, *ring_ref, event_channel, protocol;
> +    char *str;
>  
>      trace_xen_qdisk_connect(vdev->disk, vdev->partition);
> +
> +    if (xen_device_frontend_scanf(xendev, "ring-page-order", "%u",
> +                                  &order) != 1) {
> +        nr_ring_ref = 1;
> +        ring_ref = g_new(unsigned int, nr_ring_ref);
> +
> +        if (xen_device_frontend_scanf(xendev, "ring-ref", "%u",
> +                                      &ring_ref[0]) != 1) {
> +            error_setg(errp, "failed to read ring-ref");

Don't you need to free `ring_ref`?

> +            return;
> +        }
[...]

> diff --git a/include/hw/xen/xen-qdisk.h b/include/hw/xen/xen-qdisk.h
> index ade0866037..d7dd2bf0ee 100644
> --- a/include/hw/xen/xen-qdisk.h
> +++ b/include/hw/xen/xen-qdisk.h
> @@ -6,7 +6,15 @@
>  #ifndef HW_XEN_QDISK_H
>  #define HW_XEN_QDISK_H
>  
> +#include "hw/xen/xen.h"
>  #include "hw/xen/xen-bus.h"
> +#include "hw/block/block.h"
> +#include "hw/block/xen_blkif.h"
> +#include "hw/block/dataplane/xen-qdisk.h"
> +#include "sysemu/blockdev.h"
> +#include "sysemu/iothread.h"
> +#include "sysemu/block-backend.h"
> +#include "sysemu/iothread.h"

You don't need that many includes, especially not iothread.h twice ;-).

I think those new includes would be enough:
#include "hw/block/block.h"; for BlockConf
#include "sysemu/iothread.h"
#include "hw/block/dataplane/xen-qdisk.h"

>  
>  typedef enum XenQdiskVdevType {
>      XEN_QDISK_VDEV_TYPE_DP,
> @@ -33,6 +41,10 @@ typedef struct XenQdiskDevice XenQdiskDevice;
>  struct XenQdiskDevice {
>      XenDevice xendev;
>      XenQdiskVdev vdev;
> +    BlockConf conf;
> +    unsigned int max_ring_page_order;
> +    IOThread *iothread;
> +    XenQdiskDataPlane *dataplane;
>  };
>  
>  #endif /* HW_XEN_QDISK_H */

-- 
Anthony PERARD