On 5/21/19 5:54 AM, Vladimir Sementsov-Ogievskiy wrote:
> 21.05.2019 1:38, John Snow wrote:
>>
>>
>> On 5/10/19 5:11 PM, John Snow wrote:
>>> We mandate that the source node must be a root node; but there's no reason
>>> I am aware of that it needs to be restricted to such. In some cases, we need
>>> to make sure that there's a medium present, but in the general case we can
>>> allow the backup job itself to do the graph checking.
>>>
>>> This patch helps improve the error message when you try to backup from
>>> the same node more than once, which is reflected in the change to test
>>> 056.
>>>
>>> For backups with bitmaps, it will also show a better error message that
>>> the bitmap is in use instead of giving you something cryptic like "need
>>> a root node."
>>>
>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1707303
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> ---
>>> blockdev.c | 6 +++++-
>>> tests/qemu-iotests/056 | 2 +-
>>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/blockdev.c b/blockdev.c
>>> index 79fbac8450..27cb72f7aa 100644
>>> --- a/blockdev.c
>>> +++ b/blockdev.c
>>> @@ -3450,7 +3450,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup,
>>> JobTxn *txn,
>>> backup->compress = false;
>>> }
>>>
>>> - bs = qmp_get_root_bs(backup->device, errp);
>>> + bs = bdrv_lookup_bs(backup->device, backup->device, errp);
>>> if (!bs) {
>>> return NULL;
>>> }
>>> @@ -3459,6 +3459,10 @@ static BlockJob *do_drive_backup(DriveBackup
>>> *backup, JobTxn *txn,
>>> aio_context_acquire(aio_context);
>>>
>>> if (!backup->has_format) {
>>> + if (!bs->drv) {
>>> + error_setg(errp, "Device has no medium");
>>> + return NULL;
>>> + }
>>
>> Pinging my own patch with a review comment. It is weird that I shuffled
>> the error checking down below a conditional, but it's the only case
>> where we directly need do access bs->drv now.
>
> not the only: it's accessed in following
> if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
> assert(backup->format);
> if (source) {
> as well.
>
Ah, of course. Thanks.
