| Message ID | 1477928314-11184-6-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 31.10.2016 16:38, Fam Zheng wrote: > In this case we may open the source's backing image chain multiple > times. Setting share flag means the new open won't try to acquire or > check any lock, once we implement image locking. > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > An alternative is reusing (and bdrv_ref) the existing source's backing > bs instead of opening another one. If we decide that approach is better, > it's better to do it in a separate series. Yes, it is better (there's a reason why we do it for drive-mirror), and while I somehow agree that we could put it off until later, I'm not sure we should. Opening an image with both BDRV_O_RDWR and BDRV_O_SHARE_RW at the same time just because our implementation is lacking is not ideal. Anyway, the whole issue becomes more complex when involving format drivers. I'll write more to that in a response to the cover letter. > --- > blockdev.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/blockdev.c b/blockdev.c > index d11a74f..9992c5d 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3160,6 +3160,7 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp) > } > if (backup->sync == MIRROR_SYNC_MODE_NONE) { > source = bs; > + flags |= BDRV_O_SHARE_RW; In any case, there should be a comment explaining the situation here (having to go through git blame is a bit tedious...), possibly involving a TODO or FIXME regarding that we really shouldn't be using BDRV_O_SHARE_RW (or maybe we should? I'm not sure, I'll explore it in said cover letter response). Max > } > > size = bdrv_getlength(bs); >
diff --git a/blockdev.c b/blockdev.c index d11a74f..9992c5d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3160,6 +3160,7 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp) } if (backup->sync == MIRROR_SYNC_MODE_NONE) { source = bs; + flags |= BDRV_O_SHARE_RW; } size = bdrv_getlength(bs);
In this case we may open the source's backing image chain multiple times. Setting share flag means the new open won't try to acquire or check any lock, once we implement image locking. Signed-off-by: Fam Zheng <famz@redhat.com> --- An alternative is reusing (and bdrv_ref) the existing source's backing bs instead of opening another one. If we decide that approach is better, it's better to do it in a separate series. --- blockdev.c | 1 + 1 file changed, 1 insertion(+)