[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/16] qemu-char: Allow a chardev to reconnect i
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 02/16] qemu-char: Allow a chardev to reconnect if disconnected |
Date: |
Thu, 14 Nov 2013 15:56:58 +0200 |
On Wed, Nov 13, 2013 at 02:51:39PM -0600, Corey Minyard wrote:
> On 11/13/2013 02:55 AM, Gerd Hoffmann wrote:
> > Hi,
> >
> >> Allow a socket that connects to reconnect on a periodic basis if it
> >> fails to connect at startup or if the connection drops while in use.
> >> + chr->backend = i;
> >> + chr->recon_time = qemu_opt_get_number(opts, "reconnect", 0);
> >> + if (chr->recon_time) {
> >> + if (strcmp(qemu_opt_get(opts, "backend"), "socket") != 0) {
> >> + g_free(chr);
> >> + fprintf(stderr, "chardev: reconnect only supported on
> >> sockets\n");
> >> + return NULL;
> >> + }
> > I think it would work *much* better to just add a "chr_reconnect"
> > function pointer to CharDriverState. You don't need patch #1 then.
> > Also it avoids backend-specific bits in generic code. Generic code just
> > checks if chr->chr_reconnect exists to figure whenever reconnect is
> > supported or not. And the socket backend can set chr_reconnect for
> > client sockets only.
>
> I was trying for something more generic than one that just applies to
> socket devices. It may not be good for server sockets, but it might be
> useful for ptys and hotplug devices. Looking at the code, ptys already
> has its own code that does something similar; that could be pulled out
> and replaced with the generic code.
>
> Also, IMHO allocating the chardev in each back end is not optimal. It
> breaks layering. Plus patch #1 has a net reduction in lines of code
> because it pulls out all the allocations and does it in one place.
>
> Thanks,
>
> -corey
I guess this was just one suggestion.
What Gerd is asking for here is that we avoid
code like
strcmp(qemu_opt_get(opts, "backend"), "socket")
in generic code, instead making a backend report
reconnect support.
This sounds reasonable, does it not?
[Qemu-devel] [PATCH 05/16] Add a base IPMI interface, Corey Minyard, 2013/11/12
[Qemu-devel] [PATCH 07/16] ipmi: Add a KCS low-level interface, Corey Minyard, 2013/11/12
[Qemu-devel] [PATCH 06/16] ipmi: Add a PC ISA type structure, Corey Minyard, 2013/11/12
[Qemu-devel] [PATCH 08/16] ipmi: Add a BT low-level interface, Corey Minyard, 2013/11/12
[Qemu-devel] [PATCH 10/16] ipmi: Add an external connection simulation interface, Corey Minyard, 2013/11/12
[Qemu-devel] [PATCH 12/16] ipmi: Add documentation, Corey Minyard, 2013/11/12
[Qemu-devel] [PATCH 11/16] ipmi: Add tests, Corey Minyard, 2013/11/12