All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Liu <wei.liu2@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: jfehlig@suse.com, Wei Liu <wei.liu2@citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	xen-devel@lists.xen.org
Subject: Re: [PATCH 1/3] libxl, xl: don't init/dispose when not necessary
Date: Tue, 10 Feb 2015 11:39:07 +0000	[thread overview]
Message-ID: <20150210113907.GD27856@zion.uk.xensource.com> (raw)
In-Reply-To: <21721.58231.791546.413355@mariner.uk.xensource.com>

On Tue, Feb 10, 2015 at 10:54:47AM +0000, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 1/3] libxl, xl: don't init/dispose when not necessary"):
> > Functions like libxl__device_disk_from_xs_be and
> > libxl_vdev_to_device_disk should not touch the disk struct passed in.
> > It's caller's responsibility to do that.
> ...
> > Note that libxl_vdev_to_device_disk is a public API so a macro is
> > defined in libxl.h to indicate the change of behaviour.
> 
> I don't think this is really on, in the public API.
> 

Yes, it's public API -- not in libxl.h, but in libxl_utils.h. And xl is
using it.

The problem is that calling _init inside libxl_vdev_to_device_disk
violates our pattern of using libxl types.

> Why not make libxl_..._free idempotent ?  Then extra calls to _init
> are harmless.
> 

That would be a good idea and it should be done in a separate patch set.

Strictly speaking this patch is not required to fix the bug Jim spotted.
I will just drop it in this series and rework in while I make _dispose
idempotent.

Wei.

> Ian.

  reply	other threads:[~2015-02-10 11:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-09 13:21 [PATCH 0/3] Misc patches for libxl_device_disk functions Wei Liu
2015-02-09 13:21 ` [PATCH 1/3] libxl, xl: don't init/dispose when not necessary Wei Liu
2015-02-09 13:23   ` Wei Liu
2015-02-09 14:36     ` Wei Liu
2015-02-10 10:54   ` Ian Jackson
2015-02-10 11:39     ` Wei Liu [this message]
2015-02-09 13:21 ` [PATCH 2/3] libxl: factor out libxl__disk_backend_from_xs_be Wei Liu
2015-02-10 10:56   ` Ian Jackson
2015-02-10 11:39     ` Wei Liu
2015-02-09 13:21 ` [PATCH 3/3] libxl: libxl__device_from_disk should retrieve backend from xenstore Wei Liu
2015-02-10 11:01   ` Ian Jackson
2015-02-10 11:49     ` Wei Liu
2015-02-11 17:18       ` Jim Fehlig
2015-02-12 18:35         ` Ian Jackson
2015-02-23 15:13         ` Wei Liu
2015-02-26 23:49           ` Jim Fehlig
2015-03-06 13:04             ` Wei Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150210113907.GD27856@zion.uk.xensource.com \
    --to=wei.liu2@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=jfehlig@suse.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.