Re: [PATCH] libxl: do not fail device removal if backend domain is gone - Marek Marczykowski-Górecki

From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>,
	Wei Liu <wei.liu2@citrix.com>,
	xen-devel@lists.xen.org
Subject: Re: [PATCH] libxl: do not fail device removal if backend domain is gone
Date: Fri, 9 Feb 2018 16:11:06 +0100	[thread overview]
Message-ID: <20180209151105.GO2070@mail-itl> (raw)
In-Reply-To: <20180209143908.sf6lgrmpkpiggchg@MacBook-Pro-de-Roger.local>

[-- Attachment #1.1: Type: text/plain, Size: 1982 bytes --]

On Fri, Feb 09, 2018 at 02:39:08PM +0000, Roger Pau Monné wrote:
> On Fri, Feb 09, 2018 at 02:08:33PM +0100, Marek Marczykowski-Górecki wrote:
> > On Fri, Feb 09, 2018 at 12:10:39PM +0000, Roger Pau Monné wrote:
> > > On Fri, Feb 09, 2018 at 12:41:58PM +0100, Marek Marczykowski-Górecki wrote:
> > > > On Fri, Feb 09, 2018 at 11:27:04AM +0000, Roger Pau Monné wrote:
> > > > > I'm also wondering, if you jump to 'out' here, you avoid the call to
> > > > > libxl__xs_transaction_commit and instead end up calling
> > > > > libxl__xs_transaction_abort, which means the above call to
> > > > > libxl__xs_path_cleanup will not be committed to xenstore, is this
> > > > > really desired?
> > > > >
> > > > > It seems to me libxl might leak xenstore frontend entries in that
> > > > > case.
> > > > 
> > > > That call is only if aodev->force. In other cases cleanup is done in
> > > > device_hotplug_done()->libxl__device_destroy(), which have its own transaction.
> > > 
> > > Hm, right, but this would still be incorrect in the force case then?
> > > Or is this simply not needed for the 'force' case?
> > 
> > In that case, the first libxl__xs_path_cleanup will indeed be aborted.
> > But then it will be cleaned up the same way as in !force case.
> > Anyway, this is about the case when backend is already gone, so 'force'
> > doesn't really change anything - it was forcefully removed already, by
> > shutting down the backend domain (or removing backend using something
> > else)...
> 
> Are you sure? The libxl__xs_path_cleanup call is not for removing the
> backend entries but the frontend ones. Ie: the backend entries not
> being present doesn't imply the frontend entries also not being
> present.

But libxl__device_destroy do remove frontend entries.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel