Libguestfs - Apr 2019 - Re: New patch/feature for libguestfs to parse query string in URL

Hi,

sorry for the late reply, this email fall behind in my queue.

On Friday, 15 March 2019 19:00:18 CEST Chintan Patel
wrote:
> We have some change for libguestfs to include query string in URL.
This was reported years ago:
https://bugzilla.redhat.com/show_bug.cgi?id=1092583

While adding a querystring optional argument looks like an easy
solution, in reality it opens a can of worms, as it would be a generic
passthrough for options, and not really easy to handle by libguestfs.
For few more details, see also the following email with a possible way
to implement this that I wrote some time ago:
https://www.redhat.com/archives/libguestfs/2015-December/msg00016.html

One additional argument not written in the email is that using a query
string as-is works only when using the "direct" backend (i.e.
libguestfs runs qemu directly); since we support also launching qemu
using libvirt (the "libvirt" backend), this cannot work in that case.

This is why IMHO a proper fix is to
a) split the query string in its key/value components
b) pass these key/value's via the add_drive API
c) use the various parameters properly depending on the protocol of the
   disk, and on the libguestfs backend
> The changes are merged with the current Official upstream repo into fork
below.
> https://github.com/chintanrp/libguestfs
> 
> These changes help to add query string argument in add-drive and use by URI
parser.
> Please let us know if you need more details. And what are the next steps to
add these changes to upstream master?
>From a technical POV on the change itself:
- the commit message is very terse, and in case it would need a bit
  more explanation
- the new optional parameter is not documented in the documentation
  text of the add_drive API
- 'fullPath' in lib/drives.c is leaked
- the new 'query' struct member in  common/options/options.h is leaked
- use safe_asprintf instead of asprintf in lib/drives.c, so it will
  abort already in case of failure
- why was xmlURIUnescapeString() used?
- in case of nbd URIs with a socket, now the path to the socket will be
  both in the server name, and as query string; see the various tests
  in fish/test-add-uri.sh

Thanks,
-- 
Pino Toscano