xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: George Dunlap <George.Dunlap@eu.citrix.com>
To: George Dunlap <george.dunlap@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Stefano Stabellini <stefano.stabellini@citrix.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	Ian Jackson <ian.jackson@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v2 6/9] libxl: Share logic for finding path between qemuu and pygrub
Date: Tue, 22 Mar 2016 10:38:34 +0000	[thread overview]
Message-ID: <CAFLBxZYCw_uie+-=GZnkEKqUGHzC=a8a=cCm_0mVO3NSOxwXaQ@mail.gmail.com> (raw)
In-Reply-To: <1458584221-24426-7-git-send-email-george.dunlap@citrix.com>

On Mon, Mar 21, 2016 at 6:16 PM, George Dunlap <george.dunlap@citrix.com> wrote:
> From: George Dunlap <george.dunlap@eu.citrix.com>
>
> qemu can also access disks which will be provided with a qdisk backend
> directly; add a flag to libxl__device_disk_find_local_path to indicate
> whether to check for qdisk direct access.
>
> Call libxl__device_disk_find_local_path() for most paths.  If we can't
> find a local path, print an error and skip the disk, rather than using
> a bogus path.
>
> Now if there is no local access to the disk (i.e., because the disk
> has a non-local backend, or relies on a custom hotplug script), libxl
> will now print a warning and not provide the emulated disk, rather
> than providing bogus parameters to qemu which cause it to error out.
> (Such disks will still be available via the PV backend.)
>
> I left the libxl__blktap_devpath in the qemuu-specific code rather
> than sharing it with the pyrgub code because:
>
> 1) When the pygrub path runs the guest disks have not yet been set up
>
> 2) libxl__blktap_devpath() will give you the existing devpath if it
> already exists, but will set one up for you if you don't.  So on the
> pygrub path, this would end up setting up a new tap device.
>
> 3) There is no tap-specific teardown code on the pygrub path, and I
> don't want to add any (particularly since I'm hoping to remove tapdisk
> altogether soon).
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> Changes since v1:
> - Re-port on top of revisions to previous patches
> - Address line length issues
> - Remove stray space at the end of an if (xxx )
> - Break out devicemodel argument rearrangement into a separate patch
> - Spell out implications of warning messages which skip emulated disks
>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> CC: Anthony Perard <anthony.perard@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>
> ---
>  tools/libxl/libxl.c          | 17 ++++++++++++++---
>  tools/libxl/libxl_dm.c       | 27 +++++++++++++++++++++++----
>  tools/libxl/libxl_internal.h |  8 ++++++++
>  3 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2d7a154..602bec2 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -3000,8 +3000,9 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void *get_vdev_user,
>
>  /* Callbacks */
>
> -static char *libxl__device_disk_find_local_path(libxl__gc *gc,
> -                                                const libxl_device_disk *disk)
> +char *libxl__device_disk_find_local_path(libxl__gc *gc,
> +                                         const libxl_device_disk *disk,
> +                                         bool qdisk_direct)
>  {
>      char *path = NULL;
>
> @@ -3022,6 +3023,16 @@ static char *libxl__device_disk_find_local_path(libxl__gc *gc,
>          goto out;
>      }
>
> +    /*
> +     * If we're being called for a qemu path, we can pass the target
> +     * string directly as well
> +     */
> +    if (qdisk_direct && disk->backend == LIBXL_DISK_BACKEND_QDISK) {
> +        path = libxl__strdup(gc, disk->pdev_path);
> +        LOG(DEBUG, "Directly accessing local QDISK target %s", path);
> +        goto out;
> +    }
> +
>   out:
>      return path;
>  }
> @@ -3046,7 +3057,7 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
>
>      LOG(DEBUG, "Trying to find local path");
>
> -    dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk);
> +    dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk, false);
>      if (dls->diskpath) {
>          LOG(DEBUG, "Local path found, executing callback.");
>          dls->callback(egc, dls, 0);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index dfcf141..1493b84 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1182,23 +1182,42 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
>
>              if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
>                  if (!disks[i].is_cdrom) {
> -                    LOG(WARN, "cannot support"" empty disk format for %s",
> +                    LOG(WARN, "Cannot support empty disk format for %s",
>                          disks[i].vdev);
>                      continue;
>                  }
>              } else {
>                  if (format == NULL) {
>                      LOG(WARN,
> -                        "unable to determine"" disk image format %s",
> +                        "Unable to determine disk image format: %s\n",
> +                        "Disk will be available via PV drivers but not as an"
> +                        "emulated disk.",

Sorry, there's a stray comma after the first line of the multi-line
string.  Let me know if you want me to re-send the series with the
comma removed, or whether someone can take it out when committing.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-03-22 10:38 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-21 18:16 [PATCH v2 0/9] tools: Allow HVM domains emulated access to disks provided by hotplug scripts George Dunlap
2016-03-21 18:16 ` [PATCH v2 1/9] tools/hotplug: Add a "dummy" hotplug script for testing George Dunlap
2016-03-21 18:16 ` [PATCH v2 2/9] libxl: Remove redundant setting of phyical-device George Dunlap
2016-03-21 18:16 ` [PATCH v2 3/9] tools/hotplug: Write physical-device-path in addition to physical-device George Dunlap
2016-03-21 18:16 ` [PATCH v2 4/9] libxl: Move check for local access to a funciton George Dunlap
2016-03-21 18:16 ` [PATCH v2 5/9] libxl: Rearrange qemu upstream disk argument code George Dunlap
2016-03-21 18:16 ` [PATCH v2 6/9] libxl: Share logic for finding path between qemuu and pygrub George Dunlap
2016-03-22 10:38   ` George Dunlap [this message]
2016-03-21 18:16 ` [PATCH v2 7/9] libxl: Allow local access for block devices with hotplug scripts George Dunlap
2016-03-21 18:17 ` [PATCH v2 8/9] docs: Document block-script protocol George Dunlap
2016-03-22 12:52   ` Roger Pau Monné
2016-03-23 11:19     ` George Dunlap
2016-03-23 12:14       ` Roger Pau Monné
2016-03-24 16:29         ` George Dunlap
2016-03-24 16:51           ` Roger Pau Monné
2016-03-21 18:17 ` [PATCH v2 9/9] DO NOT APPLY libxl: Change hotplug script interface to use physical-device-path George Dunlap
2016-03-23 10:10 ` [PATCH v2 0/9] tools: Allow HVM domains emulated access to disks provided by hotplug scripts Roger Pau Monné
2016-03-23 10:36   ` George Dunlap

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='CAFLBxZYCw_uie+-=GZnkEKqUGHzC=a8a=cCm_0mVO3NSOxwXaQ@mail.gmail.com' \
    --to=george.dunlap@eu.citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=stefano.stabellini@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    --subject='Re: [PATCH v2 6/9] libxl: Share logic for finding path between qemuu and pygrub' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).