[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Move all the QEMU migration code to a new file



On 01/31/2011 04:06 AM, Daniel P. Berrange wrote:
>>> The functions are not moved 100%. The API entry points
>>> remain in the main QEMU driver, but once the public
>>> virDomainPtr is resolved to the internal virDomainObjPtr,
>>> all following code is moved.
>>>
>>> This will allow the new v3 API entry points to call into the
>>> same shared internal migration functions
>>>
>> The diffstat makes this look like a cleaner diff than the qemu_process
>> split, so it should be easier to review.
>>
>>> -		qemu/qemu_process.c qemu/qemu_process.h	\
>>> +		qemu/qemu_process.c qemu/qemu_process.h		\
>>> +		qemu/qemu_migration.c qemu/qemu_migration.h	\
>>
>> However, this means that you depend on the qemu_process split to happen
>> first, and that one had issues, so I'm not reviewing this one yet (but I
>> do like the idea of the patch).
> 
> The 2 line fix I posted to the other patch shouldn't
> invalidate this patch

Agreed, so I'm reviewing it now...

Another case of a missing po/POTFILES.in tweak to keep 'syntax-check' happy.

> +++ b/src/Makefile.am
> @@ -281,7 +281,8 @@ QEMU_DRIVER_SOURCES =						\
>  		qemu/qemu_hostdev.c qemu/qemu_hostdev.h		\
>  		qemu/qemu_hotplug.c qemu/qemu_hotplug.h		\
>  		qemu/qemu_conf.c qemu/qemu_conf.h		\
> -		qemu/qemu_process.c qemu/qemu_process.h	\
> +		qemu/qemu_process.c qemu/qemu_process.h		\

Is it worth squashing this whitespace tweak into the prior patch,

> +		qemu/qemu_migration.c qemu/qemu_migration.h	\

so that this commit remains just the new file?

> +
> +
> +char *qemuDomainFormatXML(struct qemud_driver *driver,
> +                          virDomainObjPtr vm,
> +                          int flags)
> +{

> +
> +        if (!(cpu = virCPUDefCopy(def_cpu))
> +            || cpuUpdate(cpu, driver->caps->host.cpu))

Unusual formatting (generally, the || is on the previous line).

>  
> -static char *qemudVMDumpXML(struct qemud_driver *driver,
> -                            virDomainObjPtr vm,
> -                            int flags)
> -{

> -        if (!(cpu = virCPUDefCopy(def_cpu))
> -            || cpuUpdate(cpu, driver->caps->host.cpu))


Oh, I see - code motion.

Also need a single line tweak for cppi, but it passed testing for me,
and was easier to review than the first.

ACK with nits (aka syntax-check) fixed.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]