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