Re: [Xen-devel] [PATCH for-4.6 5/8] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 31/07/15 17:56, Ian Campbell wrote:
> On Fri, 2015-07-31 at 17:38 +0100, Andrew Cooper wrote:
>> On 31/07/15 17:34, Ian Jackson wrote:
>>> Andrew Cooper writes ("Re: [PATCH for-4.6 5/8] tools/libxl: Save and 
>>> restore EMULATOR_XENSTORE_DATA content"):
>>>> On 29/07/15 12:49, Ian Jackson wrote:
>>>>>> +    rel_start = strlen(xs_path) - 7;
>>>>> This is horrible.
>>>> What do you recommend instead?
>>> I don't see why it is necessary to do something like rel_start at all.
>>>
>>> This whole patch could probably be made much simpler with something
>>> like
>>>
>>>    static const char *const physmap_entries[] = {
>>>        "start_addr", "size", "name", NULL
>>>    };
>>>
>>> and then loop over it a few times.
>> But the rel_path has nothing to do with which subkeys get chosen.
>>
>> It is to strip out the current domains information from
>> /local/domain/$dm_domid/device-model/$domid/.
>>
>> This is because both of the domid in the path will be different on the
>> other side of migration.  This is why EMULATOR_XENSTORE_DATA is
>> purposefully specified relative to the above path, rather than as
>> absolute paths.
> I think an approach where "/local/domain/$dm_domid/device-model/$domid/"
> was in a local const char *root and then smth like:
>
>    foreach foo(start_addr, size, name)
>         foo = gcstrcat("/physmap/", foo);
>       path = gcstrcat(root, foo)
>       fooval = read(path)
>       output(foo, fooval)
>
> With the proper GC helpers etc of course. Would be fine, no?
>
> The rel_path thing is only there because you are trying to keep "foo" and
> "root" in the same string I think.

(With the leading '/' stripped out of foo)

rel_path is to extract foo from path in O(1), and does so without two
further memory allocations.

path necessarily needs to be absolute, and the relative root wont
change, key to key.

All of this code will change anyway because of other review.  Lets see
how it ends up after that.

~Andrew

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