Re: [Xen-devel] [PATCH V2] libxl: write IO ABI for disk frontends

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Apr 25, 2013 at 09:33:23AM +0100, Ian Campbell wrote:
[...]
> > +/* get guest IO ABI */
> > +int xc_domain_get_native_protocol(xc_interface *xch,
> > +                                  uint32_t domid,
> > +                                  char **guest_abi);
> 
> With the correct placement of const in here I think you can avoid the
> potential for memory allocation failure as well as the need for the
> caller to free by just:
>       *guest_abi = XEN_IO_PROTO_ABI_xxx
> 
> Perhaps even:
>       const char *xc_domain_get_native_protocol(xch, domid)
>       ...
>               return XEN_IO_PROTO...
> 
> In the X86 case where the domctl can fail we can PERROR and return NULL,
> since the caller doesn't really care why the ABI isn't available. Or
> instead of NULL we can return the protocol of whichever bit width we are
> compiler for. Probably NULL is safer though.
> 

OK.

> > diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> > index 50853af..83cca1f 100644
> > --- a/tools/libxc/xenctrl.h
> > +++ b/tools/libxc/xenctrl.h
> > @@ -637,6 +637,18 @@ int xc_domain_hvm_setcontext(xc_interface *xch,
> >                               uint32_t size);
> >  
> >  /**
> > + * This function will return the guest word width
> > + *
> > + * @parm xch a handle to an open hypervisor interface
> > + * @parm domid the domain to get address width for
> > + * @parm guest_abi guest's navtive IO ABI string
> > + * @return 0 on success, -ev on failure
> > + */
> > +int xc_domain_get_native_protocol(xc_interface *xch,
> > +                                  uint32_t domid,
> > +                                  char **guest_abi);
> > +
> > +/**
> >   * This function returns information about the execution context of a
> >   * particular vcpu of a domain.
> >   *
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 572c2c6..3f6afad 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -2037,6 +2037,13 @@ static void device_disk_add(libxl__egc *egc, 
> > uint32_t domid,
> >      int rc;
> >      libxl_ctx *ctx = gc->owner;
> >      xs_transaction_t t = XBT_NULL;
> > +    char *protocol = NULL;
> > +
> > +    libxl_domain_type type = libxl__domain_type(gc, domid);
> 
> Did we decide that Xend only does this for PV guests? Would there be any
> harm in doing this for all PV disks, whether attached to PV or HVM
> domains? Would remove some code ;-)
> 

This string is propogated from Python libxc binding, and it is only
placed in pyxc_linux_build. So unless I'm mistaken, Xend only does this
for PV.

Removing the type check and write the node for each type of domain
should do no harm.  For newer frontends, they just blindly write that
node whether it pre-exists or not.


Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel