On 1/8/26 11:09, Richard W.M. Jones wrote:
> On Thu, Jan 08, 2026 at 09:23:23AM +0100, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <[email protected]>
>>
>> The aim of this helper function is to URI-encode given string
>> twice. There's a bug (fixed in next commit) in which we're unable
>> to fetch .vmx file for a domain if corresponding datastore
>> contains some special characters (like +). Cole Robinson
>> discovered that encoding datastore twice enables libvirt to work
>> around the issue [2]. Well, this function does exactly that.
>> It was tested with the following inputs and all worked
>> flawlessly: "datastore", "datastore2", "datastore2+",
>> "datastore3+-@", "data store2+".
>>
>> 1: https://issues.redhat.com/browse/RHEL-134127
>> 2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072
>>
>> Signed-off-by: Michal Privoznik <[email protected]>
>> ---
>>  src/esx/esx_util.c | 17 +++++++++++++++++
>>  src/esx/esx_util.h |  3 +++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
>> index 7ee0e5f7c0..e47ea36730 100644
>> --- a/src/esx/esx_util.c
>> +++ b/src/esx/esx_util.c
>> @@ -448,3 +448,20 @@ esxUtil_EscapeForXml(const char *string)
>>  
>>      return virBufferContentAndReset(&buffer);
>>  }
>> +
>> +
>> +
> 
> 3 blank lines between functions is OK??

It matches style used in the file. But yeah, I can drop this one extra
line and post a patch to drops the other extra lines.

> 
>> +/* esxUtil_EscapeInventoryObject:
>> + * @buf: the buffer to append to
>> + * @string: the string argument which will be URI-encoded
>> + *
>> + * URI-encode given @string TWICE and append the result to the @buf.
>> + */
>> +void
>> +esxUtil_EscapeInventoryObject(virBuffer *buf, const char *string)
> 
> It's nit-picky but should we explain here why we are double-encoding
> the string (ie. to workaround a VMware bug)?

It's in the commit message, but yeah, I'll add it into a comment too.
Does the following sound reasonable?

 * URI-encode given @string TWICE and append the result to the @buf. This is
 * to be used with inventory objects (like 'dcPath' and 'dsName') to work
 * around a VMware bug in which once round of URI-encoding is not enough.


> 
> But other than that, for the whole series:
> 
> Reviewed-by: Richard W.M. Jones <[email protected]>

Thanks!

Michal

Reply via email to