[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [Qemu-devel] [QEMU PATCH] block: Remove blk_attach_dev_legacy() / legacy_dev code


  • To: qemu-block@xxxxxxxxxx, Kevin Wolf <kwolf@xxxxxxxxxx>, Max Reitz <mreitz@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>
  • From: Thomas Huth <thuth@xxxxxxxxxx>
  • Date: Tue, 22 Jan 2019 15:19:35 +0100
  • Autocrypt: addr=thuth@xxxxxxxxxx; keydata= xsFNBFH7eUwBEACzyOXKU+5Pcs6wNpKzrlJwzRl3VGZt95VCdb+FgoU9g11m7FWcOafrVRwU yYkTm9+7zBUc0sW5AuPGR/dp3pSLX/yFWsA/UB4nJsHqgDvDU7BImSeiTrnpMOTXb7Arw2a2 4CflIyFqjCpfDM4MuTmzTjXq4Uov1giGE9X6viNo1pxyEpd7PanlKNnf4PqEQp06X4IgUacW tSGj6Gcns1bCuHV8OPWLkf4hkRnu8hdL6i60Yxz4E6TqlrpxsfYwLXgEeswPHOA6Mn4Cso9O 0lewVYfFfsmokfAVMKWzOl1Sr0KGI5T9CpmRfAiSHpthhHWnECcJFwl72NTi6kUcUzG4se81 O6n9d/kTj7pzTmBdfwuOZ0YUSqcqs0W+l1NcASSYZQaDoD3/SLk+nqVeCBB4OnYOGhgmIHNW 0CwMRO/GK+20alxzk//V9GmIM2ACElbfF8+Uug3pqiHkVnKqM7W9/S1NH2qmxB6zMiJUHlTH gnVeZX0dgH27mzstcF786uPcdEqS0KJuxh2kk5IvUSL3Qn3ZgmgdxBMyCPciD/1cb7/Ahazr 3ThHQXSHXkH/aDXdfLsKVuwDzHLVSkdSnZdt5HHh75/NFHxwaTlydgfHmFFwodK8y/TjyiGZ zg2Kje38xnz8zKn9iesFBCcONXS7txENTzX0z80WKBhK+XSFJwARAQABzRxUaG9tYXMgSHV0 aCA8dGguaHV0aEBnbXguZGU+wsF7BBMBAgAlAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIX gAUCUfuWKwIZAQAKCRAu2dd0/nAttbe/EACb9hafyOb2FmhUqeAiBORSsUifFacQ7laVjcgR I4um8CSHvxijYftpkM2EdAtmXIKgbNDpQoXcWLXB9lu9mLgTO4DVT00TRR65ikn3FCWcyT74 ENTOzRKyKLsDCjhXKPblTPIQbYAUCOWElcyAPm0ERd62fA/rKNxgIiNo/l4UODOMoOJm2/Ox ZoTckW68Eqv7k9L7m7j+Hn3hoDTjAmcCBJt+j7pOhzWvCbqoNOIH8C8qvPaNlrba+R/K6jkO 6jZkTbYQpGIofEQJ/TNn38IsNGpI1ALTHWFtoMxp3j2Imz0REO6dRE2fHRN8sVlHgkoeGhmY NbDsDE1jFQOEObFnu0euk//7BXU7tGOHckVAZ8T1smiRPHfQU7UEH2a/grndxJ+PNeM5w7n2 l+FN3cf2KgPotCK2s9MjSdZA7C5e3rFYO8lqiqTJKvc62vqp3e7B0Kjyy5/QtzSOejBij2QL xkKSFNtxIz4MtuxN8e3IDQNxsKry3nF7R4MDvouXlMo6wP9KuyNWb+vFJt9GtbgfDMIFVamp ZfhEWzWRJH4VgksENA4K/BzjEHCcbTUb1TFsiB1VRnBPJ0SqlvifnfKk6HcpkDk6Pg8Q5FOJ gbNHrdgXsm+m/9GF2zUUr+rOlhVbK23TUqKqPfwnD7uxjpakVcJnsVCFqJpZi1F/ga9IN87B TQRR+3lMARAAtp831HniPHb9AuKq3wj83ujZK8lH5RLrfVsB4X1wi47bwo56BqhXpR/zxPTR eOFT0gnbw9UkphVc7uk/alnXMDEmgvnuxv89PwIQX6k3qLABeV7ykJQG/WT5HQ6+2DdGtVw3 2vjYAPiWQeETsgWRRQMDR0/hwp8s8tL/UodwYCScH6Vxx9pdy353L1fK4Bb9G73a+9FPjp9l x+WwKTsltVqSBuSjyZQ3c3EE8qbTidXZxB38JwARH8yN3TX+t65cbBqLl/zRUUUTapHQpUEd yoAsHIml32e4q+3xdLtTdlLi7FgPBItSazcqZPjEcYW73UAuLcmQmfJlQ5PkDiuqcitn+KzH /1pqsTU7QFZjbmSMJyXY0TDErOFuMOjf20b6arcpEqse1V3IKrb+nqqA2azboRm3pEANLAJw iVTwK3qwGRgK5ut6N/Znv20VEHkFUsRAZoOusrIRfR5HFDxlXguAdEz8M/hxXFYYXqOoaCYy 6pJxTjy0Y/tIfmS/g9Bnp8qg9wsrsnk0+XRnDVPak++G3Uq9tJPwpJbyO0vcqEI3vAXkAB7X VXLzvFwi66RrsPUoDkuzj+aCNumtOePDOCpXQGPpKl+l1aYRMN/+lNSk3+1sVuc2C07WnYyE gV/cbEVklPmKrNwu6DeUyD0qI/bVzKMWZAiB1r56hsGeyYcAEQEAAcLBXwQYAQIACQUCUft5 TAIbDAAKCRAu2dd0/nAttYTwEACLAS/THRqXRKb17PQmKwZHerUvZm2klo+lwQ3wNQBHUJAT p2R9ULexyXrJPqjUpy7+voz+FcKiuQBTKyieiIxO46oMxsbXGZ70o3gxjxdYdgimUD6U8PPd JH8tfAL4BR5FZNjspcnscN2jgbF4OrpDeOLyBaj6HPmElNPtECHWCaf1xbIFsZxSDGMA6cUh 0uX3Q8VI7JN1AR2cfiIRY7NrIlWYucJxyKjO3ivWm69nCtsHiJ0wcF8KlVo7F2eLaufo0K8A ynL8SHMF3VEyxsXOP2f1UR9T2Ur30MXcTBpjUxml1TX3RWY5uH89Js/jlIugBwuAmacJ7JYh lTg6sF/GNc4nPb4kk2yktNWTade+TzsllYlJPaorD2Qe8qX0iFUhFC6y9+O6mP4ZvWoYapp9 ezYNuebMgEr93ob1+4sFg3812wNP01WqsGtWCJHnPv/JoonFdMzD/bIkXGEJMk6ks2kxQQZq g6Ik/s/vxOfao/xCn8nHt7GwvVy41795hzK6tbSl+BuyCRp0vfPRP34OnK7+jR2nvQpJu/pU rCELuGwT9hsYkUPjVd4lfylN3mzEc6iAv/wwjsc0DRTSQCpXT3v2ymTAsRKrVaEZLibTXaf+ WslxWek3xNYRiqwwWAJuL652eAlxUgQ5ZS+fXBRTiQpJ+F26I/2lccScRd9G5w==
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, paul.durrant@xxxxxxxxxx, qemu-devel@xxxxxxxxxx
  • Delivery-date: Tue, 22 Jan 2019 14:19:44 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 2018-12-18 17:11, Thomas Huth wrote:
> The last user of blk_attach_dev_legacy() is the code in xen_disk.c.
> It passes a pointer to a XenBlkDev as second parameter. XenBlkDev
> is derived from XenDevice which in turn is derived from DeviceState
> since commit 3a6c9172ac5951e ("xen: create qdev for each backend device").
> Thus the code can also simply use blk_attach_dev() with a pointer
> to the DeviceState instead.
> So we can finally remove all code related to the "legacy_dev" flag, too,
> and turn the related "void *" in block-backend.c into "DeviceState *"
> to fix some of the remaining TODOs there.
> 
> Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx>
> ---
>  Note: I haven't tested the Xen code since I don't have a working Xen
>  installation at hand. I'd appreciate if someone could check it...

Ping?

>  block/block-backend.c          | 54 
> +++++++-----------------------------------
>  hw/block/xen_disk.c            |  6 +++--
>  include/sysemu/block-backend.h |  5 ++--
>  3 files changed, 15 insertions(+), 50 deletions(-)
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 60d37a0..3c3118f 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -47,9 +47,7 @@ struct BlockBackend {
>      QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
>      BlockBackendPublic public;
>  
> -    void *dev;                  /* attached device model, if any */
> -    bool legacy_dev;            /* true if dev is not a DeviceState */
> -    /* TODO change to DeviceState when all users are qdevified */
> +    DeviceState *dev;           /* attached device model, if any */
>      const BlockDevOps *dev_ops;
>      void *dev_opaque;
>  
> @@ -836,7 +834,11 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, 
> uint64_t *shared_perm)
>      *shared_perm = blk->shared_perm;
>  }
>  
> -static int blk_do_attach_dev(BlockBackend *blk, void *dev)
> +/*
> + * Attach device model @dev to @blk.
> + * Return 0 on success, -EBUSY when a device model is attached already.
> + */
> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
>  {
>      if (blk->dev) {
>          return -EBUSY;
> @@ -851,40 +853,16 @@ static int blk_do_attach_dev(BlockBackend *blk, void 
> *dev)
>  
>      blk_ref(blk);
>      blk->dev = dev;
> -    blk->legacy_dev = false;
>      blk_iostatus_reset(blk);
>  
>      return 0;
>  }
>  
>  /*
> - * Attach device model @dev to @blk.
> - * Return 0 on success, -EBUSY when a device model is attached already.
> - */
> -int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> -{
> -    return blk_do_attach_dev(blk, dev);
> -}
> -
> -/*
> - * Attach device model @dev to @blk.
> - * @blk must not have a device model attached already.
> - * TODO qdevified devices don't use this, remove when devices are qdevified
> - */
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev)
> -{
> -    if (blk_do_attach_dev(blk, dev) < 0) {
> -        abort();
> -    }
> -    blk->legacy_dev = true;
> -}
> -
> -/*
>   * Detach device model @dev from @blk.
>   * @dev must be currently attached to @blk.
>   */
> -void blk_detach_dev(BlockBackend *blk, void *dev)
> -/* TODO change to DeviceState *dev when all users are qdevified */
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev)
>  {
>      assert(blk->dev == dev);
>      blk->dev = NULL;
> @@ -898,8 +876,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
>  /*
>   * Return the device model attached to @blk if any, else null.
>   */
> -void *blk_get_attached_dev(BlockBackend *blk)
> -/* TODO change to return DeviceState * when all users are qdevified */
> +DeviceState *blk_get_attached_dev(BlockBackend *blk)
>  {
>      return blk->dev;
>  }
> @@ -908,10 +885,7 @@ void *blk_get_attached_dev(BlockBackend *blk)
>   * device attached to the BlockBackend. */
>  char *blk_get_attached_dev_id(BlockBackend *blk)
>  {
> -    DeviceState *dev;
> -
> -    assert(!blk->legacy_dev);
> -    dev = blk->dev;
> +    DeviceState *dev = blk->dev;
>  
>      if (!dev) {
>          return g_strdup("");
> @@ -949,11 +923,6 @@ BlockBackend *blk_by_dev(void *dev)
>  void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
>                       void *opaque)
>  {
> -    /* All drivers that use blk_set_dev_ops() are qdevified and we want to 
> keep
> -     * it that way, so we can assume blk->dev, if present, is a DeviceState 
> if
> -     * blk->dev_ops is set. Non-device users may use dev_ops without device. 
> */
> -    assert(!blk->legacy_dev);
> -
>      blk->dev_ops = ops;
>      blk->dev_opaque = opaque;
>  
> @@ -979,8 +948,6 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool 
> load, Error **errp)
>          bool tray_was_open, tray_is_open;
>          Error *local_err = NULL;
>  
> -        assert(!blk->legacy_dev);
> -
>          tray_was_open = blk_dev_is_tray_open(blk);
>          blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err);
>          if (local_err) {
> @@ -1779,9 +1746,6 @@ void blk_eject(BlockBackend *blk, bool eject_flag)
>      BlockDriverState *bs = blk_bs(blk);
>      char *id;
>  
> -    /* blk_eject is only called by qdevified devices */
> -    assert(!blk->legacy_dev);
> -
>      if (bs) {
>          bdrv_eject(bs, eject_flag);
>      }
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 36eff94..9605caf 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -801,7 +801,9 @@ static int blk_connect(struct XenDevice *xendev)
>           * so we can blk_unref() unconditionally */
>          blk_ref(blkdev->blk);
>      }
> -    blk_attach_dev_legacy(blkdev->blk, blkdev);
> +    if (blk_attach_dev(blkdev->blk, DEVICE(blkdev)) < 0) {
> +        return -1;
> +    }
>      blkdev->file_size = blk_getlength(blkdev->blk);
>      if (blkdev->file_size < 0) {
>          BlockDriverState *bs = blk_bs(blkdev->blk);
> @@ -951,7 +953,7 @@ static void blk_disconnect(struct XenDevice *xendev)
>  
>      if (blkdev->blk) {
>          blk_set_aio_context(blkdev->blk, qemu_get_aio_context());
> -        blk_detach_dev(blkdev->blk, blkdev);
> +        blk_detach_dev(blkdev->blk, DEVICE(blkdev));
>          blk_unref(blkdev->blk);
>          blkdev->blk = NULL;
>      }
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index c96bcde..39507d6 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -110,9 +110,8 @@ void blk_iostatus_disable(BlockBackend *blk);
>  void blk_iostatus_reset(BlockBackend *blk);
>  void blk_iostatus_set_err(BlockBackend *blk, int error);
>  int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
> -void blk_attach_dev_legacy(BlockBackend *blk, void *dev);
> -void blk_detach_dev(BlockBackend *blk, void *dev);
> -void *blk_get_attached_dev(BlockBackend *blk);
> +void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
> +DeviceState *blk_get_attached_dev(BlockBackend *blk);
>  char *blk_get_attached_dev_id(BlockBackend *blk);
>  BlockBackend *blk_by_dev(void *dev);
>  BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
> 


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.