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

[Chrysostomos Nanakos]

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 <address@hidden>
> > ---
> >   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.