On 06/26/2018 09:38 PM, Eric Blake wrote:
> On 06/26/2018 05:22 PM, John Snow wrote:
>> In the case of image fleecing, the node we choose as the source
>> for a blockdev-backup is going to be both a root node AND the
>> backing node for the exported image. It does not qualify as a root
>> image in this case.
>>
>> Loosen the restriction.
>
> Did we regress (and if so, when), or has this never worked? But you are
> right: visually, we are starting with:
It's actually a regression, because I had a variant of this test in my
local files that used to work. We never really advertised or tested this
feature, though, so ...
>
> Device
> Base (backing) <- Top (active)
>
> then want to add nodes for an NBD export:
> Device NBD
> Base (backing) <- Top <- Tmp
[device] --> [active]
^
[nbd] --> [fleecing]------'
So active here has two parents: the device root, and the temporary
fleecing node. This disqualifies it from being the source in
blockdev_backup, but there's no real reason to.
If the caller of blockdev_backup chooses a dumb source node they'll get
dumb data. GIGO.
>
> with a blockdev-backup "sync":"none" from Top to Tmp (any writes from
> the Device first copy the old data to Tmp; the NBD export sees a
> read-only view of Tmp that is unchanging from the time the backup job
> started, regardless of what the Device does in the meantime).
>
> Then when the fleece job ends, the NBD export is stopped, the
> blockdev-backup job is canceled, and Tmp is thrown away as unneeded.
>
Yup, highlighted a little better in the next patch.
>>
>> Signed-off-by: John Snow <[email protected]>
>> ---
>> blockdev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 58d7570932..526f8b60be 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -3517,7 +3517,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup
>> *backup, JobTxn *txn,
>> backup->compress = false;
>> }
>> - bs = qmp_get_root_bs(backup->device, errp);
>> + bs = bdrv_lookup_bs(backup->device, backup->device, errp);
>
> Reviewed-by: Eric Blake <[email protected]>
>
>> if (!bs) {
>> return NULL;
>> }
>>
>
