All of
 help / color / mirror / Atom feed
From: Ian Campbell <>
To: Jim Fehlig <>
Cc: Ian Jackson <>,
	Wei Liu <>,
	"" <>
Subject: Re: [PATCH RFC] libxl: set disk defaults in remove/destroy functions
Date: Mon, 2 Feb 2015 13:36:00 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Mon, 2015-02-02 at 13:27 +0000, Ian Campbell wrote:
> On Mon, 2015-01-26 at 16:14 -0700, Jim Fehlig wrote:
> Cc-ing the other toolstack maintainers, both of whom have more
> familiarity with this part of libxl than I.
> > The attached patch is a hack I cooked up to fix one of the libvirt-TCK
> > Xen failures.  The test (200-disk-hotplug.t) attempts to hot add and
> > remove a disk from a running domain.  The add works fine, but remove
> > fails with
> > 
> > libxl: debug: libxl.c:3858:libxl_device_disk_remove: ao 0x7f9b9c0015a0:
> > create: how=(nil) callback=(nil) poller=
> > 0x7f9ba0004590
> > libxl: error: libxl.c:2399:libxl__device_from_disk: unrecognized disk
> > backend type: 0
> > 
> > The test does not define a backend type, in which case the libvirt libxl
> > driver allows libxl to choose an appropriate backend via
> > libxl__device_disk_set_backend().  The backend type is never set on a
> > remove operation, hence it fails with the above error.
> > 
> > I spent some time trying to figure out the best place to set backend
> > type on remove, but in the end could only come up with this hack.  It
> > wouldn't feel so gross if I could have simply added
> > 'libxl__device_##type##_setdefault(gc, type);' to the existing
> > DEFINE_DEVICE_REMOVE macro, but alas libxl__device_nic_setdefault() has
> > a different prototype than the other devices.

(I'm not sure this is the right apporach, but if it were...)

FWIW libxl__ functions aren't stable, so in theory this could be changed
to rationalise them all (e.g. by adding domid to the rest), but that
would be a bit churnful.

Perhaps better would be to add a new parameter to0 DEFINE_DEVICE_REMOVE
like extra_setdefault_args which is pasted in the appropriate place?

> > Better suggestions welcomed!  One I considered was fixing this in
> > libvirt.  But the Xen community suggested allowing libxl to choose a
> > suitable backend when not specified, so I think this recommendation
> > should be symmetrical in the add and remove operations.

I suppose on remove its not so much a case of choosing a suitable
backend as reflecting the actual current reality for that device. Which
suggests that libxl__device_from_disk ought to be figuring out the
actual backend somehow rather than being told it.


  reply	other threads:[~2015-02-02 13:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-26 23:14 [PATCH RFC] libxl: set disk defaults in remove/destroy functions Jim Fehlig
2015-02-02 13:27 ` Ian Campbell
2015-02-02 13:36   ` Ian Campbell [this message]
2015-02-02 13:41     ` Ian Jackson
2015-02-02 16:32   ` Wei Liu
2015-02-02 16:38     ` Ian Jackson
2015-02-02 18:00       ` Wei Liu
2015-02-02 18:06         ` Ian Jackson
2015-02-02 18:13           ` Wei Liu
2015-02-03 11:27             ` Ian Jackson

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:

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

  git send-email \ \ \ \ \ \ \

* 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.