On Mon, Feb 02, 2015 at 01:11:07PM +0100, Pino Toscano wrote:

Parse the query string from URLs, and pass it as new add_drive optarg; use it when building the file= URL for qemu.

Accept query string only for http and https protocols, for now. --- Possibly it looks like an ad-hoc solution for http(s), although I'm not sure how it could possibly be generalized somehow (maybe "extra params" which would be the query string for http(s)?).

A few comments: It needs test case(s) in fish/test-add-uri.sh. I think in the API and code we should either consistently call it 'query' or 'querystring' (whichever, but consistently). What's quoting/escaping does the querystring require? Probably the caller should not have to quote it, but then there are two problems: (a) does the quoting need to be removed when we parse the guestfish URL and (b) does quoting need to be added when we call curl at the API level? Rich.

customize/customize_main.ml | 4 ++-- fish/options.c | 6 ++++++ fish/options.h | 1 + fish/uri.c | 27 ++++++++++++++++++++++++--- fish/uri.h | 1 + generator/actions.ml | 11 ++++++++++- mllib/uRI.ml | 1 + mllib/uRI.mli | 1 + mllib/uri-c.c | 13 ++++++++++++- resize/resize.ml | 4 ++-- src/drives.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/guestfs-internal.h | 2 ++ src/launch-direct.c | 24 +++++++++++++----------- sysprep/main.ml | 4 ++-- 14 files changed, 118 insertions(+), 22 deletions(-)

diff --git a/customize/customize_main.ml b/customize/customize_main.ml index 5bba71a..9b9613f 100644 --- a/customize/customize_main.ml +++ b/customize/customize_main.ml @@ -167,12 +167,12 @@ read the man page virt-customize(1). fun (uri, format) -> let { URI.path = path; protocol = protocol; server = server; username = username; - password = password } = uri in + password = password; query = query } = uri in let discard = if readonly then None else Some "besteffort" in g#add_drive ~readonly ?discard ?format ~protocol ?server ?username ?secret:password - path + ?querystring:query path ) files in

diff --git a/fish/options.c b/fish/options.c index 9ffcc6b..0d5ec2e 100644 --- a/fish/options.c +++ b/fish/options.c @@ -68,6 +68,7 @@ option_a (const char *arg, const char *format, struct drv **drvsp) drv->uri.server = uri.server; drv->uri.username = uri.username; drv->uri.password = uri.password; + drv->uri.query = uri.query; drv->uri.format = format; drv->uri.orig_uri = arg; } @@ -172,6 +173,10 @@ add_drives_handle (guestfs_h *g, struct drv *drv, char next_drive) ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK; ad_optargs.secret = drv->uri.password; } + if (drv->uri.query) { + ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_QUERYSTRING_BITMASK; + ad_optargs.querystring = drv->uri.query; + }

r = guestfs_add_drive_opts_argv (g, drv->uri.path, &ad_optargs); if (r == -1) @@ -307,6 +312,7 @@ free_drives (struct drv *drv) guestfs___free_string_list (drv->uri.server); free (drv->uri.username); free (drv->uri.password); + free (drv->uri.query); break; case drv_d: /* d.filename is optarg, don't free it */ diff --git a/fish/options.h b/fish/options.h index cf68122..d4b0fa5 100644 --- a/fish/options.h +++ b/fish/options.h @@ -73,6 +73,7 @@ struct drv { char **server; /* server(s) - can be NULL */ char *username; /* username - can be NULL */ char *password; /* password - can be NULL */ + char *query; /* query - can be NULL */ const char *format; /* format (NULL == autodetect) */ const char *orig_uri; /* original URI (for error messages etc.) */ } uri; diff --git a/fish/uri.c b/fish/uri.c index f45c907..8459a7c 100644 --- a/fish/uri.c +++ b/fish/uri.c @@ -34,7 +34,7 @@ #include "uri.h"

static int is_uri (const char *arg); -static int parse (const char *arg, char **path_ret, char **protocol_ret, char ***server_ret, char **username_ret, char **password_ret); +static int parse (const char *arg, char **path_ret, char **protocol_ret, char ***server_ret, char **username_ret, char **password_ret, char **query_ret); static char *query_get (xmlURIPtr uri, const char *search_name); static int make_server (xmlURIPtr uri, const char *socket, char ***ret);

@@ -46,10 +46,11 @@ parse_uri (const char *arg, struct uri *uri_ret) char **server = NULL; char *username = NULL; char *password = NULL; + char *query = NULL;

/* Does it look like a URI? */ if (is_uri (arg)) { - if (parse (arg, &path, &protocol, &server, &username, &password) == -1) + if (parse (arg, &path, &protocol, &server, &username, &password, &query) == -1) return -1; } else { @@ -72,6 +73,7 @@ parse_uri (const char *arg, struct uri *uri_ret) uri_ret->server = server; uri_ret->username = username; uri_ret->password = password; + uri_ret->query = query; return 0; }

@@ -101,7 +103,8 @@ is_uri (const char *arg)

static int parse (const char *arg, char **path_ret, char **protocol_ret, - char ***server_ret, char **username_ret, char **password_ret) + char ***server_ret, char **username_ret, char **password_ret, + char **query_ret) { CLEANUP_XMLFREEURI xmlURIPtr uri = NULL; CLEANUP_FREE char *socket = NULL; @@ -201,6 +204,24 @@ parse (const char *arg, char **path_ret, char **protocol_ret, return -1; }

+ *query_ret = NULL; + /* Copy the query string part only when not building a unix: URI + * (e.g. for nbd). See logic done in make_server. + */ + if (uri->query_raw && STRNEQ (uri->query_raw, "") && + !(socket && (uri->server == NULL || STREQ (uri->server, "")))) { + *query_ret = strdup (uri->query_raw); + if (*query_ret == NULL) { + perror ("strdup: query"); + free (*protocol_ret); + guestfs___free_string_list (*server_ret); + free (*username_ret); + free (*password_ret); + free (*path_ret); + return -1; + } + } + return 0; }

diff --git a/fish/uri.h b/fish/uri.h index 9202a70..d9b100a 100644 --- a/fish/uri.h +++ b/fish/uri.h @@ -27,6 +27,7 @@ struct uri { char **server; /* server(s) - can be NULL */ char *username; /* username - can be NULL */ char *password; /* password - can be NULL */ + char *query; /* query string - can be NULL */ };

/* Parse the '-a' option parameter 'arg', and place the result in diff --git a/generator/actions.ml b/generator/actions.ml index c0beaae..18cbfc0 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -1318,7 +1318,7 @@ not all belong to a single logical operating system

{ defaults with name = "add_drive"; - style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"]; + style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"; OString "querystring"]; once_had_no_optargs = true; blocking = false; fish_alias = ["add"]; @@ -1575,6 +1575,15 @@ of the same area of disk.

The default is false.

+=item C<querystring> + +The string parameter C<querystring> specifies the optional query string +to be used when accessing some kind of resources, for example: + + https://example.org/images/?image=test1 + +in such cases, C<image=test1> needs to passed as C<querystring>. + =back" };

{ defaults with diff --git a/mllib/uRI.ml b/mllib/uRI.ml index d4f7522..9ee53ee 100644 --- a/mllib/uRI.ml +++ b/mllib/uRI.ml @@ -22,6 +22,7 @@ type uri = { server : string array option; username : string option; password : string option; + query : string option; }

external parse_uri : string -> uri = "virt_resize_parse_uri" diff --git a/mllib/uRI.mli b/mllib/uRI.mli index 0692f95..7fb9acd 100644 --- a/mllib/uRI.mli +++ b/mllib/uRI.mli @@ -24,6 +24,7 @@ type uri = { server : string array option; (** list of servers *) username : string option; (** username *) password : string option; (** password *) + query : string option; (** query string *) }

val parse_uri : string -> uri diff --git a/mllib/uri-c.c b/mllib/uri-c.c index aa63c48..e649cdb 100644 --- a/mllib/uri-c.c +++ b/mllib/uri-c.c @@ -49,7 +49,7 @@ virt_resize_parse_uri (value argv /* arg value, not an array! */) caml_invalid_argument ("URI.parse_uri");

/* Convert the struct into an OCaml tuple. */ - rv = caml_alloc_tuple (5); + rv = caml_alloc_tuple (6);

/* path : string */ sv = caml_copy_string (uri.path); @@ -94,5 +94,16 @@ virt_resize_parse_uri (value argv /* arg value, not an array! */) ov = Val_int (0); Store_field (rv, 4, ov);

+ /* query : string option */ + if (uri.query) { + sv = caml_copy_string (uri.query); + free (uri.query); + ov = caml_alloc (1, 0); + Store_field (ov, 0, sv); + } + else + ov = Val_int (0); + Store_field (rv, 5, ov); + CAMLreturn (rv); } diff --git a/resize/resize.ml b/resize/resize.ml index 871c6a4..2a2765c 100644 --- a/resize/resize.ml +++ b/resize/resize.ml @@ -338,8 +338,8 @@ read the man page virt-resize(1). if verbose then g#set_verbose true; let _, { URI.path = path; protocol = protocol; server = server; username = username; - password = password } = infile in - g#add_drive ?format ~readonly:true ~protocol ?server ?username ?secret:password path; + password = password; query = query } = infile in + g#add_drive ?format ~readonly:true ~protocol ?server ?username ?secret:password ?querystring:query path; (* The output disk is being created, so use cache=unsafe here. *) g#add_drive ?format:output_format ~readonly:false ~cachemode:"unsafe" outfile; diff --git a/src/drives.c b/src/drives.c index 34bf63d..3668e29 100644 --- a/src/drives.c +++ b/src/drives.c @@ -63,6 +63,7 @@ struct drive_create_data { const char *cachemode; enum discard discard; bool copyonread; + const char *query; };

COMPILE_REGEXP (re_hostname_port, "(.*):(\\d+)$", 0) @@ -144,6 +145,7 @@ create_drive_non_file (guestfs_h *g, drv->src.username = data->username ? safe_strdup (g, data->username) : NULL; drv->src.secret = data->secret ? safe_strdup (g, data->secret) : NULL; drv->src.format = data->format ? safe_strdup (g, data->format) : NULL; + drv->src.query = data->query ? safe_strdup (g, data->query) : NULL;

drv->readonly = data->readonly; drv->iface = data->iface ? safe_strdup (g, data->iface) : NULL; @@ -171,6 +173,13 @@ static struct drive * create_drive_curl (guestfs_h *g, const struct drive_create_data *data) { + if (data->query != NULL && + data->protocol != drive_protocol_http && + data->protocol != drive_protocol_https) { + error (g, _("curl: you cannot specify a query string with this protocol")); + return NULL; + } + if (data->nr_servers != 1) { error (g, _("curl: you must specify exactly one server")); return NULL; @@ -207,6 +216,10 @@ create_drive_gluster (guestfs_h *g, error (g, _("gluster: you cannot specify a secret with this protocol")); return NULL; } + if (data->query != NULL) { + error (g, _("gluster: you cannot specify a query string with this protocol")); + return NULL; + }

if (data->nr_servers != 1) { error (g, _("gluster: you must specify exactly one server")); @@ -250,6 +263,10 @@ create_drive_nbd (guestfs_h *g, error (g, _("nbd: you cannot specify a secret with this protocol")); return NULL; } + if (data->query != NULL) { + error (g, _("nbd: you cannot specify a query string with this protocol")); + return NULL; + }

if (data->nr_servers != 1) { error (g, _("nbd: you must specify exactly one server")); @@ -268,6 +285,11 @@ create_drive_rbd (guestfs_h *g, { size_t i;

+ if (data->query != NULL) { + error (g, _("rbd: you cannot specify a query string with this protocol")); + return NULL; + } + for (i = 0; i < data->nr_servers; ++i) { if (data->servers[i].transport != drive_transport_none && data->servers[i].transport != drive_transport_tcp) { @@ -307,6 +329,10 @@ create_drive_sheepdog (guestfs_h *g, error (g, _("sheepdog: you cannot specify a secret with this protocol")); return NULL; } + if (data->query != NULL) { + error (g, _("sheepdog: you cannot specify a query string with this protocol")); + return NULL; + }

for (i = 0; i < data->nr_servers; ++i) { if (data->servers[i].transport != drive_transport_none && @@ -337,6 +363,11 @@ static struct drive * create_drive_ssh (guestfs_h *g, const struct drive_create_data *data) { + if (data->query != NULL) { + error (g, _("ssh: you cannot specify a query string with this protocol")); + return NULL; + } + if (data->nr_servers != 1) { error (g, _("ssh: you must specify exactly one server")); return NULL; @@ -379,6 +410,10 @@ create_drive_iscsi (guestfs_h *g, error (g, _("iscsi: you cannot specify a secret with this protocol")); return NULL; } + if (data->query != NULL) { + error (g, _("iscsi: you cannot specify a query string with this protocol")); + return NULL; + }

if (data->nr_servers != 1) { error (g, _("iscsi: you must specify exactly one server")); @@ -770,6 +805,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, ? optargs->secret : NULL; data.cachemode = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK ? optargs->cachemode : NULL; + data.query = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_QUERYSTRING_BITMASK + ? optargs->querystring : NULL;

if (optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK) { if (STREQ (optargs->discard, "disable")) @@ -835,6 +872,10 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, error (g, _("you cannot specify a secret with file-backed disks")); return -1; } + if (data.query != NULL) { + error (g, _("you cannot specify a query string with file-backed disks")); + return -1; + }

if (STREQ (filename, "/dev/null")) drv = create_drive_dev_null (g, &data); diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 573c3da..439a80b 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -242,6 +242,8 @@ struct drive_source { char *username; /* Optional secret (may be NULL if not specified). */ char *secret; + /* Optional query string (may be NULL if not specified). */ + char *query; };

enum discard { diff --git a/src/launch-direct.c b/src/launch-direct.c index e6ed54a..00f3190 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -1189,7 +1189,8 @@ qemu_escape_param (guestfs_h *g, const char *param) static char * make_uri (guestfs_h *g, const char *scheme, const char *user, const char *password, - struct drive_server *server, const char *path) + struct drive_server *server, const char *path, + const char *querystring) { xmlURI uri = { .scheme = (char *) scheme, .user = (char *) user }; @@ -1217,6 +1218,7 @@ make_uri (guestfs_h *g, const char *scheme, const char *user, case drive_transport_tcp: uri.server = server->u.hostname; uri.port = server->port; + uri.query_raw = (char *) querystring; break; case drive_transport_unix: query = safe_asprintf (g, "socket=%s", server->u.socket); @@ -1258,36 +1260,36 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src)

case drive_protocol_ftp: return make_uri (g, "ftp", src->username, src->secret, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, NULL);

case drive_protocol_ftps: return make_uri (g, "ftps", src->username, src->secret, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, NULL);

case drive_protocol_gluster: switch (src->servers[0].transport) { case drive_transport_none: return make_uri (g, "gluster", NULL, NULL, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, NULL); case drive_transport_tcp: return make_uri (g, "gluster+tcp", NULL, NULL, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, NULL); case drive_transport_unix: return make_uri (g, "gluster+unix", NULL, NULL, - &src->servers[0], NULL); + &src->servers[0], NULL, NULL); }

case drive_protocol_http: return make_uri (g, "http", src->username, src->secret, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, src->query);

case drive_protocol_https: return make_uri (g, "https", src->username, src->secret, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, src->query);

case drive_protocol_iscsi: return make_uri (g, "iscsi", NULL, NULL, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, NULL);

case drive_protocol_nbd: { CLEANUP_FREE char *p = NULL; @@ -1374,11 +1376,11 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src)

case drive_protocol_ssh: return make_uri (g, "ssh", src->username, src->secret, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, NULL);

case drive_protocol_tftp: return make_uri (g, "tftp", src->username, src->secret, - &src->servers[0], src->u.exportname); + &src->servers[0], src->u.exportname, NULL); }

abort (); diff --git a/sysprep/main.ml b/sysprep/main.ml index 249800f..ff9852c 100644 --- a/sysprep/main.ml +++ b/sysprep/main.ml @@ -201,12 +201,12 @@ read the man page virt-sysprep(1). fun (uri, format) -> let { URI.path = path; protocol = protocol; server = server; username = username; - password = password } = uri in + password = password; query = query } = uri in let discard = if readonly then None else Some "besteffort" in g#add_drive ~readonly ?discard ?format ~protocol ?server ?username ?secret:password - path + ?querystring:query path ) files in

-- 1.9.3

_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs

-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top