[PATCH 2/2 for 8.0] qemu: add index for isa-serial device using target.port

Divya Garg divya.garg at nutanix.com
Thu Jan 13 13:32:32 UTC 2022


On 13/01/22 6:47 pm, Ani Sinha wrote:
>
>
> On Thu, Jan 13, 2022 at 4:50 PM John Levon <levon at movementarian.org> 
> wrote:
>
>     On Thu, Jan 13, 2022 at 03:11:51PM +0530, Ani Sinha wrote:
>
>     > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>     > > index d822533ccb..4130df0ed9 100644
>     > > --- a/src/qemu/qemu_command.c
>     > > +++ b/src/qemu/qemu_command.c
>     > > @@ -10718,6 +10718,8 @@ qemuBuildSerialChrDeviceProps(const
>     virDomainDef *def,
>     > >      g_autoptr(virJSONValue) props = NULL;
>     > >      g_autofree char *chardev = g_strdup_printf("char%s",
>     serial->info.alias);
>     > >      virQEMUCapsFlags caps;
>     > > +    const char *typestr;
>     > > +    int ret;
>     >
>     > type should match the return type of this function.
>
>     It does match return type of
>     virDomainChrSerialTargetModelTypeToString():
>
>      47 #define VIR_ENUM_DECL(name) \
>      48     const char *name ## TypeToString(int type); \ 
>
>
> Yes you are correct. I was thinking of qemuBuildSerialChrDeviceProps() 
> but it does not return ret directly from this function. Another reason 
> it makes me uncomfortable.
>
>
>     > I preferred your previous style to this one.
>
>     Below is cleaner IMO: we don't repeat code, and the flow is much
>     clearer.
>
>
> If you look at other functions for example qemuBuildVirtioDevProps() 
> etc they follow the previous style. I think it's fine to return NULL 
> from multiple points.
> In any case if the maintainers are fine with it , I'm ok. Style is 
> trivial.

Thankyou Ani ! Let me know if I need to update anything else for the patch.

Thanks :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220113/95ee4ce9/attachment-0001.htm>


More information about the libvir-list mailing list