Re: [Qemu-devel] [PATCH v6 3/5] block/archipelago: Add support for creating images

[Chrysostomos Nanakos <cnanakos@grnet.gr>]

On 07/02/2014 05:01 PM, Eric Blake wrote:
> On 06/27/2014 02:24 AM, Chrysostomos Nanakos wrote:
>> qemu-img archipelago:<volumename>[/mport=<mapperd_port>[:vport=<vlmcd_port>]
>>   [:segment=<segment_name>]] [size]
>>
>> Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
>> ---
>>   block/archipelago.c |  149 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 149 insertions(+)
>>
>> +
>> +    /* Try default values if none has been set */
>> +    if (mportno == (xport) -1) {
>> +        mportno = 1001;
>> +    }
>> +
>> +    if (vportno == (xport) -1) {
>> +        vportno = 501;
>> +    }
> I've now seen these magic numbers in more than one patch; it's worth a
> #define or enum name.

Yes for sure. I will use two #define's for that.

>> +
>> +    if (segment_name == NULL) {
>> +        segment_name = g_strdup("archipelago");
>> +    }
>> +
>> +    if (xseg_initialize()) {
>> +        error_setg(errp, "Cannot initialize XSEG");
>> +        return -1;
>> +    }
>> +
>> +    xseg = xseg_join((char *)"posix", segment_name,
>> +                     (char *)"posixfd", NULL);
> Do you really have to cast away const to use this interface?  Have you
> reported that as a design bug to the authors of xseg_join?

I will remove casting and fix xseg_join.

Thanks.