From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:57656)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cminyard@mvista.com>) id 1VghPr-0000p7-Iu
	for qemu-devel@nongnu.org; Wed, 13 Nov 2013 15:52:06 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <cminyard@mvista.com>) id 1VghPi-00022P-Kb
	for qemu-devel@nongnu.org; Wed, 13 Nov 2013 15:51:59 -0500
Received: from mail-yh0-f50.google.com ([209.85.213.50]:62690)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cminyard@mvista.com>) id 1VghPi-00020J-Gf
	for qemu-devel@nongnu.org; Wed, 13 Nov 2013 15:51:50 -0500
Received: by mail-yh0-f50.google.com with SMTP id b12so534853yha.37
	for <qemu-devel@nongnu.org>; Wed, 13 Nov 2013 12:51:49 -0800 (PST)
Message-ID: <5283E65B.9020908@mvista.com>
Date: Wed, 13 Nov 2013 14:51:39 -0600
From: Corey Minyard <cminyard@mvista.com>
MIME-Version: 1.0
References: <1384273995-16486-1-git-send-email-cminyard@mvista.com>	
	<1384273995-16486-3-git-send-email-cminyard@mvista.com>
	<1384332943.3560.18.camel@nilsson.home.kraxel.org>
In-Reply-To: <1384332943.3560.18.camel@nilsson.home.kraxel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 02/16] qemu-char: Allow a chardev to
 reconnect if disconnected
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gerd Hoffmann <kraxel@redhat.com>, Corey Minyard <minyard@acm.org>
Cc: Bret Ketchum <bcketchum@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, qemu-devel@nongnu.org, =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= <afaerber@suse.de>

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