On 4/4/23 20:10, skran wrote:
> Thank you for the corrections. I understand it now.
> [1] I have made the Graphics Transport validation function static and moved its call out of the parser, to virDomainGraphicsDefListensValidate() in domain_validate.h. Is this a correct implementation?, it does pass the tests.
> I have removed the *graphics def parameter from ParseXML function as it became unused after moving the validation out of the Parser.
>
> [2] I can't find a way to implement virDomainStorageNetHostSocketValidate() as static. Could you please point as to which function within domain_validate.h could be used to call this validation function? similar to virDomainGraphicsDefListensValidate() for [1].
>
> [3] How do I obtain char *transport within the validation function without parameter passing?
> It's used only to report this error in the below snippet
> virReportError(VIR_ERR_XML_ERROR,
> _("transport '%1$s' does not support socket attribute"),
> transport);
> Do I use virXMLPropString(hostnode, "transport") after passing xmlNodePtr hostnode as a parameter? How do I obtain these in domain_validate.h.
This is more correct yes. It moves at least one of the functions to the
correct place and calls it from the correct place. But not the other, is
there a reason for that?
Also, you are still missing couple of points raised earlier - the commit
message is rather poor. This is not what anybody would like to see in
git log. And since this is the third time I'm raising this objection and
it falls flat, I'm given no other option than not review next version if
the commit message is not fixed. Sorry.
And lastly, we do not send patches against earlier versions of patches.
Just send the patch again. What I usually do when given review (assume
my patches are on branch "my_fixes":
git checkout -b my_fixes_v2 my_fixes
git rebase -i master; # here I change all 'pick' to 'edit' (or just 'e')
# and change individual commits as requested in
# review, and once I'm done:
git format-patch -v2 master # if there are two or more patches I also
# pass --cover-letter
At this point I edit formatted patch(es), in a very careful way: ....
>
>
> Thank you for your help!
> Signed-off-by: K Shiva <shiva_kr@xxxxxxxxxx>
> ---
... you see these three dashes ^^^? This is the place where I put:
This is a v2 of:
$(paste url of my previous patch from the libvir-list archive here)
diff to v1:
- renamed variable in virFunctionXYZ()
- fixed memory leak,
- other things I've done.
Now, if there were more patches than just one, I've covered all of this
on the cover letter instead of individual patches.
Here's an example of v2 for multiple patches:
https://listman.redhat.com/archives/libvir-list/2023-February/237635.html
and here's an example of v2 for a single patch:
https://listman.redhat.com/archives/libvir-list/2022-December/236463.html
Michal
