[Qemu-block] [PATCH 2/3] block-backend: Remember if attached device is non-qdev
Almost all block devices are qdevified by now. This allows us to go back from the BlockBackend to the DeviceState. xen_disk is the last device that is missing. We'll remember in the BlockBackend if a xen_disk is attached and can then disable any features that require going from a BB to the DeviceState. While at it, clearly mark the function used by xen_disk as legacy even in its name, not just in TODO comments. Signed-off-by: Kevin Wolf --- block/block-backend.c | 26 +++--- hw/block/xen_disk.c| 2 +- include/sysemu/block-backend.h | 4 ++-- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 911254a..39c5613 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -38,6 +38,7 @@ struct BlockBackend { 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 */ const BlockDevOps *dev_ops; void *dev_opaque; @@ -507,32 +508,38 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs) } } -/* - * Attach device model @dev to @blk. - * Return 0 on success, -EBUSY when a device model is attached already. - */ -int blk_attach_dev(BlockBackend *blk, void *dev) -/* TODO change to DeviceState *dev when all users are qdevified */ +static int blk_do_attach_dev(BlockBackend *blk, void *dev) { if (blk->dev) { return -EBUSY; } 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_nofail(BlockBackend *blk, void *dev) +void blk_attach_dev_legacy(BlockBackend *blk, void *dev) { if (blk_attach_dev(blk, dev) < 0) { abort(); } +blk->legacy_dev = true; } /* @@ -586,6 +593,11 @@ 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 is a DeviceState if blk->dev_ops + * is set. */ +assert(!blk->legacy_dev); + blk->dev_ops = ops; blk->dev_opaque = opaque; } diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 5aa350a..1292a4b 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -1079,7 +1079,7 @@ static int blk_connect(struct XenDevice *xendev) * so we can blk_unref() unconditionally */ blk_ref(blkdev->blk); } -blk_attach_dev_nofail(blkdev->blk, blkdev); +blk_attach_dev_legacy(blkdev->blk, blkdev); blkdev->file_size = blk_getlength(blkdev->blk); if (blkdev->file_size < 0) { BlockDriverState *bs = blk_bs(blkdev->blk); diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index a7993af..b07159b 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -107,8 +107,8 @@ BlockDeviceIoStatus blk_iostatus(const BlockBackend *blk); 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, void *dev); -void blk_attach_dev_nofail(BlockBackend *blk, void *dev); +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); BlockBackend *blk_by_dev(void *dev); -- 1.8.3.1
Re: [Qemu-block] [PATCH 2/3] block-backend: Remember if attached device is non-qdev
On 05.10.2016 11:26, Kevin Wolf wrote: > Almost all block devices are qdevified by now. This allows us to go back > from the BlockBackend to the DeviceState. xen_disk is the last device > that is missing. We'll remember in the BlockBackend if a xen_disk is > attached and can then disable any features that require going from a BB > to the DeviceState. > > While at it, clearly mark the function used by xen_disk as legacy even > in its name, not just in TODO comments. > > Signed-off-by: Kevin Wolf > --- > block/block-backend.c | 26 +++--- > hw/block/xen_disk.c| 2 +- > include/sysemu/block-backend.h | 4 ++-- > 3 files changed, 22 insertions(+), 10 deletions(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index 911254a..39c5613 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c [...] > @@ -507,32 +508,38 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState > *bs) > } > } > > -/* > - * Attach device model @dev to @blk. > - * Return 0 on success, -EBUSY when a device model is attached already. > - */ > -int blk_attach_dev(BlockBackend *blk, void *dev) > -/* TODO change to DeviceState *dev when all users are qdevified */ > +static int blk_do_attach_dev(BlockBackend *blk, void *dev) > { > if (blk->dev) { > return -EBUSY; > } > 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_nofail(BlockBackend *blk, void *dev) > +void blk_attach_dev_legacy(BlockBackend *blk, void *dev) > { > if (blk_attach_dev(blk, dev) < 0) { I'd make this a blk_do_attach_dev(), but it's not syntactically wrong, so the choice is up to you. Thus, either way: Reviewed-by: Max Reitz > abort(); > } > +blk->legacy_dev = true; > } > > /* signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH 2/3] block-backend: Remember if attached device is non-qdev
On 10/05/2016 01:01 PM, Max Reitz wrote: >> +static int blk_do_attach_dev(BlockBackend *blk, void *dev) >> + */ >> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev) >> +{ >> +return blk_do_attach_dev(blk, dev); >> +void blk_attach_dev_legacy(BlockBackend *blk, void *dev) >> { >> if (blk_attach_dev(blk, dev) < 0) { > > I'd make this a blk_do_attach_dev(), but it's not syntactically wrong, > so the choice is up to you. It's technically undefined C behavior to cast from void* to an unrelated pointer and back to void* (you are only guaranteed a round trip from type to void* and back to original type, not with unrelated types). So syntactically valid but semantically undefined. On the other hand, ABI-wise, I'd be shocked if our compilers are able to exploit our use of undefined behavior. At any rate, using blk_do_attach_dev() would avoid the definedness problem, so I'd go ahead and make the change. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH 2/3] block-backend: Remember if attached device is non-qdev
Am 05.10.2016 um 20:01 hat Max Reitz geschrieben: > On 05.10.2016 11:26, Kevin Wolf wrote: > > Almost all block devices are qdevified by now. This allows us to go back > > from the BlockBackend to the DeviceState. xen_disk is the last device > > that is missing. We'll remember in the BlockBackend if a xen_disk is > > attached and can then disable any features that require going from a BB > > to the DeviceState. > > > > While at it, clearly mark the function used by xen_disk as legacy even > > in its name, not just in TODO comments. > > > > Signed-off-by: Kevin Wolf > > +/* > > + * 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_nofail(BlockBackend *blk, void *dev) > > +void blk_attach_dev_legacy(BlockBackend *blk, void *dev) > > { > > if (blk_attach_dev(blk, dev) < 0) { > > I'd make this a blk_do_attach_dev(), but it's not syntactically wrong, > so the choice is up to you. Thanks for catching this, what you suggest is what I intended. Maybe we should just enable the gcc --do-what-i-mean option. Kevin pgpJdrkx7tdF_.pgp Description: PGP signature