From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH v8 3/5] libxl: add support for vscsi Date: Mon, 15 Feb 2016 15:52:29 +0000 Message-ID: <20160215155229.GD10489@citrix.com> References: <1455205411-25460-1-git-send-email-olaf@aepfle.de> <1455205411-25460-4-git-send-email-olaf@aepfle.de> <20160212172748.GA8818@citrix.com> <20160212182459.GA28852@aepfle.de> <20160215151610.GC10489@citrix.com> <20160215152438.GA9048@aepfle.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20160215152438.GA9048@aepfle.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Olaf Hering Cc: Ian Jackson , Stefano Stabellini , Wei Liu , Ian Campbell , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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