> On 9/27/2016 2:52 PM, Paolo Bonzini wrote: > > On 27/09/2016 13:37, Roy Shterman wrote: > > > > > + iscsi_url = iscsi_parse_full_url(iscsi, > > > > > uri_string_unescape(filename, -1, NULL)); > > > > > if (iscsi_url == NULL) { > > > > > - error_setg(errp, "Failed to parse URL : %s", filename); > > > > > + error_setg(errp, "Failed to parse URL : %s", > > > > > uri_string_unescape(filename, -1, NULL)); > > > > > > > > uri_string_unescape() returns a newly allocated string. This is a > > > > memory leak! > > > > Is unescaping a bug fix? Please put it into a separate patch. > > > > > > because libvirt is parsing '?' char as %3F, I needed to parse to URI > > > with unescaping. > > > > This looks like a libvirt bug. But if libvirt learns to pass iser:// > > URIs, the unescape is not necessary, is it? > > right, but because libvirt inbox versions doesn't support > protocol_name=iser I decided to leave both options available. > The ?iser option and the iser:// option, to also have compatibility with > inbox Libvirt versions.
No, please don't do that, especially because unescaping the URI is wrong: if somebody puts a %2F in the libvirt XML it should _not_ separate the IQN from the LUN number for example. Paolo