From mboxrd@z Thu Jan  1 00:00:00 1970
From: Kamala Narasimhan <kamala.narasimhan@gmail.com>
Subject: Re: [PATCH 3/5] xl disk configuration parsing changes
Date: Tue, 08 Feb 2011 14:08:52 -0500
Message-ID: <4D5194C4.3060709@gmail.com>
References: <4D506309.7010803@gmail.com>
	<alpine.DEB.2.00.1102081552330.7277@kaball-desktop>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <alpine.DEB.2.00.1102081552330.7277@kaball-desktop>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
List-Id: xen-devel@lists.xenproject.org

>> -#define DSTATE_INITIAL   0
>> -#define DSTATE_TAP       1
>> -#define DSTATE_PHYSPATH  2
>> -#define DSTATE_VIRTPATH  3
>> -#define DSTATE_VIRTTYPE  4
>> -#define DSTATE_RW        5
>> -#define DSTATE_TERMINAL  6
>> +#define DSTATE_INITIAL           0
>> +#define DSTATE_ATTRIB_PARSED     1
>> +#define DSTATE_VDEV_PARSED       2
> 
> Why did you need to remove 4 states?
I didn't have a need for it.

> Also it would be nice to split this patch in two: a patch that
> introduces parse_disk_attrib, parse_disk_vdev_info, etc, and a second
> patch that makes any necessary changes to the state machine.
> Otherwise it is very difficult to understand the reason behind each
> change to the state machine.
>
That might be a little tricky.  I will talk to you on this offline.

>>  static int parse_disk_config(libxl_device_disk *disk, char *buf2)
>>  {
> 
> There doesn't seem to be a way to set DISK_FORMAT_EMPTY anymore, in fact
> a config line like ',hdc:cdrom,w' seems to be broken.
> 
Patch 5/5, validation code should take care of it.

Kamala