From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kamala Narasimhan Subject: Re: [RFC] xl disk configuration handling Date: Mon, 31 Jan 2011 15:19:33 -0500 Message-ID: <4D471955.8070103@gmail.com> References: <4D45A410.4000304@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Stefano Stabellini Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org > I agree that libxl_disk_phystype expresses both the format and the > backend type in a single confusing way, so there should be two enums: > one for the format (libxl_disk_format) and one for the backend type > (libxl_disk_pdev_type). I will switch to two enums instead of three. > However I don't think libxl_disk_impl_type should be exposed at all, it > should be up to libxl to decide whether AIO should be enabled or not. It > might be useful to let the user disable the PV interface for a > particular disk (that is what ioemu stands for), but it is too late for > 4.1, let's just ignore ioemu for the moment. Ok. > The backend types should be BLKBACK, TAPDISK2, and QEMU; TAPDISK refers > to blktap1 that is not supported by libxl. However libxl uses "tap:" as > backend string corresponding to TAPDISK2, I understand that might be > confusing but I wouldn't change it now. > Also it might be useful to retain the EMPTY format among the various > libxl_disk_format's, it should reduce the overall amount of changes. > EMPTY, an indicator that there is no media in the cd-rom drive didn't really go with the any of the enums which is why I removed it. But later when I was changing code I did find it inconvenient to check for both empty path plus cdrom, so I will add it to disk format types though I am really not sure if it belongs there. > it would be nice if all the renaming was done in a separate patch so > that the real changes are easier to read > I was worried you may not accept a patch with just renaming changes! I could separate interface changes (which would include renaming) from parsing and send them as two separate patches. Would that be ok? > Stefano - I did go through your comments on a subset of code here but as I mentioned in my earlier email, please ignore that code for now as I was going to modify it anyway. It was mostly to help understand the places that require change plus for the code to compile. > > do we really need to change the parsing function that much? I > understand there are significant changes but this is a total rewrite. > I am concerned about all the bugs we might find later after the > release... > This is one change I would really like to go with. Not only does it help with the changes we needed, it also gets rid of code duplication. With this change block-attach can rely on the same parsing code (that is once I submit the block-attach changes patch). > > I would completely ignore "aio:" here. > I would also ignore "ioemu:" the same way. > This redundant logic in block attach for parsing will be gone and disk parsing logic will be reused. Kamala