Re: [PATCH v8 3/5] libxl: add support for vscsi

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Feb 15, 2016 at 04:24:38PM +0100, Olaf Hering wrote:
> On Mon, Feb 15, Wei Liu wrote:
> 
> > > I think yes, DEFINE_DEVICES_ADD has to be used somewhere.
> > I'm confused. You're joking, right? "Has to be used somewhere" is not
> > a justification for having it in this particular place.
> 
> What would be the appropriate place? I think its there since I started
> working on it at 4.4 times.
>

Depending on what you want to do.

For example, if you grep for libxl__add_nics (defined with that macro)
you can see it being called in libxl_create.c at a later point and
libxl_dm.c when configuring backend QEMU for stubdom.

So there is two aspects of this:

1. At which point do you want the attachment to happen?
2. How should this device attachment be done?

For #1, you need to look on a higher level than that function you
modified in the patch. In you patch, the attachment is done at the
same time disks are attached before creating device model.  So my
first reply was to ask why you would that to happen at that
particular point.

For #2, in you patch you lump disk and vscsi together in one step. I
think multidev can handle your changes just fine. But I'm not sure if
that's the best practice -- for example, if either of them fails, you
can only get error message "unable to add disk devices". You can
introduce yet another step in the callback chain.

Wei.

> Olaf