Re: [Xen-devel] [PATCH V4 22/24] xl: update domain configuration when we hotplug a device

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, May 06, 2014 at 05:57:34PM +0100, David Vrabel wrote:
> On 01/05/14 13:58, Wei Liu wrote:
> > A convenient macro is written to accomplish following tasks:
> > 
> > 1. load domain configuration
> > 2. allocate a new device structure NEW
> > 3. copy the parsed device strcture PARSED to NEW
> > 4. call libxl_device_TYPE_add(PARSED)
> > 5. pull from PARSED any fields that might be touched by libxl to NEW
> > 6. store domain configuration
> > 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  tools/libxl/xl_cmdimpl.c |   85 
> > +++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 69 insertions(+), 16 deletions(-)
> > 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 83f058e..91172c5 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -381,6 +381,36 @@ static void *xrealloc(void *ptr, size_t sz) {
> >          &(array)[array_extend_old_count];                               \
> >      })
> >  
> > +#define ARRAY_EXTEND_INIT_NODEVID(array,count,initfn)                   \
> > +    ({                                                                  \
> > +        typeof((count)) array_extend_old_count = (count);               \
> > +        (count)++;                                                      \
> > +        (array) = xrealloc((array), sizeof(*array) * (count));          \
> > +        (initfn)(&(array)[array_extend_old_count]);                     \
> > +        &(array)[array_extend_old_count];                               \
> > +    })
> > +
> > +/* Add a device and update the stored configuration */
> > +#define ADD_DEVICE(devtype,domid,dev,d_config,ptr,allocate,maybe_fixup) \
> > +    do {                                                                \
> > +        libxl_domain_config_init((d_config));                           \
> > +        load_domain_config((domid), (d_config));                        \
> > +                                                                        \
> > +        allocate;                                                       \
> > +                                                                        \
> > +        libxl_device_ ## devtype ## _copy(ctx,(ptr),&(dev));            \
> > +                                                                        \
> > +        if (libxl_device_ ## devtype ## _add(ctx,(domid),&(dev), 0)) {  \
> > +            fprintf(stderr, "libxl_device_%s_add failed.\n", #devtype); \
> > +            exit(1);                                                    \
> > +        }                                                               \
> > +                                                                        \
> > +        maybe_fixup;                                                    \
> > +                                                                        \
> > +        store_domain_config((domid), (d_config));                       \
> > +        libxl_domain_config_dispose((d_config));                        \
> > +    } while (0)
> > +
> 
> Is there a better way of doing this without these obtuse macros?
> 

Do you not like macro? Or do you not like this be done in xl?

> Is libxl lacking is a proper device abstraction layer?
> 

libxl does have a device abstraction layer, but it cannot help much
here. Not quite sure what "proper" means.

It's done like this (regardless putting this macro in xl or libxl)
because the library device abstraction layer (for now) by itself doesn't
know what to preserve, what information is critical to restore a domain.

Wei.

> David

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