From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 1/5] libxl: remove DM path in libxl__device_model_destroy Date: Wed, 18 Mar 2015 13:21:03 +0000 Message-ID: <1426684863.14291.24.camel@citrix.com> References: <1426242875-24446-1-git-send-email-wei.liu2@citrix.com> <1426242875-24446-2-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1426242875-24446-2-git-send-email-wei.liu2@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: anthony.perard@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Fri, 2015-03-13 at 10:34 +0000, Wei Liu wrote: > ... because it is the right place to clean up device model stuffs. ... and not devices_destroy_cb because it is the right ... (also "stuff"). > And the path should use LIBXL_TOOLSTACK_DOMID instead of hardcoded 0. Between this and what is in the next patch I think you probably should refactor into a helper function to get the correct path for a given domid. FWIW I think you might also be able to do: GCSPRINTF("/local/domain/" #LIBXL_TOOLSTACK_DOMID "/device-model/%d", domid); If you want, although perhaps the intention is for it to eventually not be a hard-define of 0 and become e.g. a function call or a global variable reference, in which case best not? > Signed-off-by: Wei Liu > --- > tools/libxl/libxl.c | 2 -- > tools/libxl/libxl_dm.c | 6 ++++++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 088786e..46a43d5 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -1659,8 +1659,6 @@ static void devices_destroy_cb(libxl__egc *egc, > > xs_rm(ctx->xsh, XBT_NULL, libxl__xs_libxl_path(gc, domid)); > xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc, > - "/local/domain/0/device-model/%d", domid)); > - xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc, > "/local/domain/%d/hvmloader", domid)); > > /* This is async operation, we already hold CTX lock */ > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 8599a6a..0fd5ffa 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -1629,6 +1629,12 @@ out: > > int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid) > { > + char *path = GCSPRINTF("/local/domain/%d/device-model/%d", > + LIBXL_TOOLSTACK_DOMID, domid); > + > + if (!xs_rm(CTX->xsh, XBT_NULL, path)) > + LOG(ERROR,"xs_rm failed for %s", path); > + /* We should try to destroy the device model anyway. */ > return kill_device_model(gc, > GCSPRINTF("/local/domain/%d/image/device-model-pid", domid)); > }