Re: [libvirt] [PATCH 07/12] qemu: introduce 2 func qemuDomainRNGInsert and qemuDomainRNGRemove
On 01/05/2015 11:22 PM, Peter Krempa wrote:
On 01/03/15 06:06, Luyao Huang wrote:
> qemu side functions, call virDomainRNGInsert and virDomainRNGRemove
> to help us.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/qemu/qemu_hotplug.c | 23 +++++++++++++++++++++++
> src/qemu/qemu_hotplug.h | 7 +++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7f93b9b..f9327b4 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1501,6 +1501,29 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
> return ret;
> }
>
> +int
> +qemuDomainRNGInsert(virDomainDefPtr vmdef,
> + virDomainRNGDefPtr rng)
> +{
> + return virDomainRNGInsert(vmdef, rng);
> +}
This wrapper doesn't seem useful.
Yes, i will remove this function in next
version.
> +
> +virDomainRNGDefPtr
> +qemuDomainRNGRemove(virDomainDefPtr vmdef,
> + virDomainRNGDefPtr rng)
> +{
> + virDomainRNGDefPtr ret;
> +
> + if (!(ret = virDomainRNGRemove(vmdef, rng))) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("device not present in domain configuration"));
> + return NULL;
> + }
Given that this function is used exactly once in the series you've
posted it doesn't make much sense to have the code separate.
Okay, ...
> +
> + return ret;
> +}
> +
> +
> static int
> qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
> index d13c532..7b838ee 100644
> --- a/src/qemu/qemu_hotplug.h
> +++ b/src/qemu/qemu_hotplug.h
> @@ -107,6 +107,13 @@ virDomainChrDefPtr
> qemuDomainChrRemove(virDomainDefPtr vmdef,
> virDomainChrDefPtr chr);
>
> +int
> +qemuDomainRNGInsert(virDomainDefPtr vmdef,
> + virDomainRNGDefPtr rng);
> +virDomainRNGDefPtr
> +qemuDomainRNGRemove(virDomainDefPtr vmdef,
> + virDomainRNGDefPtr rng);
> +
Both of the functions above are used only in qemu_hotplug.c. It doesn't
make sense to export them.
... i will remove them and won't export
qemuDomainRNGRemove.
Thanks for your review and pointing out.
> void qemuDomainRemoveDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainDeviceDefPtr dev);
>
Peter