Re: [Qemu-devel] [PATCH v2 1/1] block: Support Archipelago as a QEMU block backend

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]

On 06/02/2014 09:49 AM, Chrysostomos Nanakos wrote:

>>>> I still think that there is no need to burden the code with this
>>>> shorthand...
>>> If everyone agrees I can totally remove that part.
>> I don't think it's a bad idea to provide a shortcut for human users. The
>> important point is just that internally it's not the main interface, but
>> mapped to the structured option (which I think your patch achieves now).

I'm okay if you go with Kevin's vote for keeping the shorthand in place.

>>>>>   block/archipelago.c | 1174
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> This is a lot to review at once - is there any reasonable way to break
>>>> it into a series of smaller patches?
>>> It will be really difficult to break it in smaller patches, and I
>>> believe it will more difficult for someone to review it.
>> Not sure how much easier the review would become, but the obvious split
>> that works almost always is to do a read-only driver in patch 1, and
>> then add write and create support in patches 2 and 3.
> 
> I can do that. Eric do you think that I should do it now and resend the
> patches or in the next version in which I will add support for the QMP
> blockdev-add command?

Up to you.  I'm fine with waiting for a v3 that has everything (QMP and
split along Kevin's proposal of read-only vs. write/create) rather than
list churn of an immediate resend.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]