Re: [libvirt] [PATCH 3/3] util: add backing store parser support for gluster protocol
On Mon, Oct 12, 2015 at 09:14:33 -0400, Prasanna Kumar Kalever wrote:
(Missing reply header?)
> > On Thu, Oct 08, 2015 at 17:25:53 +0530, Prasanna Kumar
Kalever wrote:
(I'd like to strongly suggest that you trim down the responses to the
relevant portions only ... )
[...]
> > On the other hand, it might be worth checking where the
backing chain
> > functions are used and just move all the backing store parsers into qemu
> > code as probably all of it is very qemu specific too.
> >
Can you please tell me into which file is it meaningful to push
"virStorageSourceParseBackingJSON" under src/qemu or do you prefer to create
a new file with name src/qemu/qemu_parse_json.c ?
I've thought about that a bit more and for now we can go with leaving it
in the common utils file. Despite being qemu specific, the storage
driver uses the APIs too and we should be able to use the backing files
created by qemu in the storage driver too so please disregard this point
for now.
> > > +{
> > > +#if WITH_STORAGE_GLUSTER
[...]
> > Wrong field name. This is also the reason why
"volfile-servers" is a
> > terrible name. Any other protocol that certainly won't call it's
server
> > field volfile-servers. We would need to add a different code path here.
> >
> > Gluster isn't the only storage technology on the planet, so this code
> > needs to be extensible
>
> Peter, I have send the patch to qemu just before with the naming conventions
> yourself and Kevin suggested on v5 patch of qemu.
> But before sending this patch qemu v5 patch-set have the older naming
> conventions,
> so obviously if somebody want to test this patch which depends on qemu patch,
> the naming conventions must be as qemu v5 path-set, so knowingly I followed
> the
> older naming conventions. I hope you understand that part.
Well, since you've posted this after you've received the review on the
qemu part where it was almost certain that the design of the qemu part
would change it didn't make much sense to post this since it wasted your
time writing the code and even if the rest of the code would be in good
shape you'd still need to post another version with the changed naming.
Peter
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)