[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 1/3] block: add target-id option to drive-ba
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/3] block: add target-id option to drive-backup QMP command |
Date: |
Thu, 27 Jun 2013 12:57:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 |
Il 27/06/2013 11:41, Fam Zheng ha scritto:
> On Thu, 06/27 10:15, Stefan Hajnoczi wrote:
>> On Wed, Jun 26, 2013 at 11:59:19AM +0800, Fam Zheng wrote:
>>> Add target-id (optional) to drive-backup command, to make the target bs
>>> a named drive so that we can operate on it (e.g. export with NBD).
>>>
>>> Signed-off-by: Fam Zheng <address@hidden>
>>> ---
>>> blockdev.c | 4 +++-
>>> qapi-schema.json | 7 +++++--
>>> qmp-commands.hx | 3 ++-
>>> 3 files changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/blockdev.c b/blockdev.c
>>> index b3a57e0..5e694f3 100644
>>> --- a/blockdev.c
>>> +++ b/blockdev.c
>>> @@ -935,6 +935,7 @@ static void drive_backup_prepare(BlkTransactionState
>>> *common, Error **errp)
>>> backup = common->action->drive_backup;
>>>
>>> qmp_drive_backup(backup->device, backup->target,
>>> + backup->has_target_id, backup->target_id,
>>> backup->has_format, backup->format,
>>> backup->has_mode, backup->mode,
>>> backup->has_speed, backup->speed,
>>> @@ -1420,6 +1421,7 @@ void qmp_block_commit(const char *device,
>>> }
>>>
>>> void qmp_drive_backup(const char *device, const char *target,
>>> + bool has_target_id, const char *target_id,
>>> bool has_format, const char *format,
>>> bool has_mode, enum NewImageMode mode,
>>> bool has_speed, int64_t speed,
>>> @@ -1494,7 +1496,7 @@ void qmp_drive_backup(const char *device, const char
>>> *target,
>>> return;
>>> }
>>>
>>> - target_bs = bdrv_new("");
>>> + target_bs = bdrv_new(has_target_id ? target_id : "");
>>
>> This raises a new issue:
>>
>> Now that the target can be named, what happens when the user issues a
>> monitor command, e.g. drive-del, block-resize, or drive-backup :)?
>>
>> We have a clumsy form of protection with bdrv_set_in_use(). It makes
>> several monitor commands refuse with -EBUSY.
>>
>> Perhaps we should have a command permission set so it's possible to
>> allow/deny specific commands.
>>
>
> Yes, this makes me realize that ref count it not a solution to retire
> bs->in_use, because we can't tell if drive-del or block-resize is safe
> with only reference number. But I can't think of two situations to deny
> different subsets of commands, shouldn't a general blocker, like in_use
> does, be good enough?
For example, right now nbd-server-add does not check bdrv_in_use. But
shrinking a device that is exposed via NBD could be surprising to the
NBD clients.
Paolo
[Qemu-devel] [RFC PATCH 2/3] block: assign backing relationship in drive-backup, Fam Zheng, 2013/06/26
[Qemu-devel] [RFC PATCH 3/3] nbd: don't get ref if bs has no drive, Fam Zheng, 2013/06/26
Re: [Qemu-devel] [RFC PATCH 0/3] Point-in-time snapshot exporting with drive-backup, Paolo Bonzini, 2013/06/26