Re: [libvirt] [PATCH] libxl: implement virDomainObjCheckIsActive
On 02/13/2017 01:24 PM, Jim Fehlig wrote:
Sagar Ghuge wrote:
> Hi Jim,
>
> Thank you for replying. I am planning to participate in GSoC'17 and
> this is my first patch towards it. This is from Byte sized task which
> I picked up and trying to solve it.
Ah, yes, this one
http://wiki.libvirt.org/page/BiteSizedTasks#Add_function_that_raises_erro...
> Yes you are right, pattern is used
> throughout the libxl driver and many others but this is just a
> rudimentary patch which will help me to get suggestion that am I on
> right path. if yes then I can I can go ahead and make changes
> accordingly.
It looks good to me, although I'm not sure how to best split up the work. Maybe
a patch that adds virDomainObjCheckIsActive, and then a patch for each driver
that replaces virDomainObjIsActive with virDomainObjCheckIsActive?
AFAICT, Cole added that task to the wiki. Perhaps he has a suggestion on the
best way to organize the work.
Sorry for the late response.
For these types of patches, please link to the BiteSizedTask in the mail after
the -- break generated by git format-patch: this keeps the link out of the
commit message since it's not really relevant there, but gives more info to
potential patch reviewers.
For this item I'd say do a patch series containing more instances of
conversions, to show that it's actually simplifying a common code pattern. So
maybe a patch series like:
1) Add the function
2) Convert src/test/*
3) Convert src/libxl/*
4) Convert src/xen/*
That shouldn't be too much work and will give a bit more to comment on, and if
there's no objections, send the remaining conversions as an additional patch
series.
- Cole