Message ID | 87lhpb7l0l.fsf@blackfin.pond.sub.org |
---|---|
State | New |
Headers | show |
On 22.09.2014 17:06, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> On 16.09.2014 20:12, Markus Armbruster wrote: >>> Commit 2d246f0 introduced DriveInfo member enable_auto_del to >>> distinguish DriveInfo created via drive_new() from DriveInfo created >>> via qmp_blockdev_add(). The latter no longer exist. Drop >>> enable_auto_del. >>> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> blockdev.c | 11 +++-------- >>> include/sysemu/blockdev.h | 1 - >>> 2 files changed, 3 insertions(+), 9 deletions(-) >> I would've liked some comment somewhere about DriveInfo's presence >> corresponding with the drive having been created through drive_new(), >> but I can live without, too. > Fair request. I could squash the following into PATCH 18: > > diff --git a/block/block-backend.c b/block/block-backend.c > index 5646628..5e4ecd7 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -22,7 +22,7 @@ struct BlockBackend { > char *name; > int refcnt; > BlockDriverState *bs; > - DriveInfo *legacy_dinfo; > + DriveInfo *legacy_dinfo; /* null unless created by drive_new() */ > QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */ > > void *dev; /* attached device model, if any */ Yes, that would be nice, thank you! >>> diff --git a/blockdev.c b/blockdev.c >>> index 0d99be0..e218c59 100644 >>> --- a/blockdev.c >>> +++ b/blockdev.c >> [snip] >> >>> @@ -1727,8 +1723,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) >>> } >>> bs = blk_bs(blk); >>> - dinfo = blk_legacy_dinfo(blk); >>> - if (dinfo && !dinfo->enable_auto_del) { >>> + if (!blk_legacy_dinfo(blk)) { >>> error_report("Deleting device added with blockdev-add" >>> " is not supported"); >>> return -1; >> This doesn't look like a 1-to-1 correspondence. Before this patch, if >> DriveInfo was not present, the condition was false (actually, it was >> always false, which is the reason for this patch). Now it's true. It >> seems like the behavior is now correct but wasn't before... I guess >> this means patch 18 should be fixed? > PATCH 18 is broken. See also my reply to Fam on v2 of this patch. Hm, okay. Fine with me. I don't object to squashing 19 into 18, either. Max >> However, for this patch: >> >> Reviewed-by: Max Reitz <mreitz@redhat.com> > Thanks!
diff --git a/block/block-backend.c b/block/block-backend.c index 5646628..5e4ecd7 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -22,7 +22,7 @@ struct BlockBackend { char *name; int refcnt; BlockDriverState *bs; - DriveInfo *legacy_dinfo; + DriveInfo *legacy_dinfo; /* null unless created by drive_new() */ QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */ void *dev; /* attached device model, if any */