On 2018-12-18 19:33, Markus Armbruster wrote: > Thomas Huth <th...@redhat.com> writes: > >> 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 <th...@redhat.com> >> --- >> 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... [...] >> 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; >> + } > > Other error returns in this function call xen_pv_printf() first. Should > this one, too?
Only some of them do a xen_pv_printf() first, there are also many that don't. blk_attach_dev() currently only returns an error if a device has already been attach - which should simply never happen here, so a printf currently does not seem to be justified to me. Alternatively, we could also abort() here instead, I think. I'll leave that decision up to the Xen maintainers... Thomas _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel