[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1 of 4 v2] libxl: Combine device model arg build functions
On Fri, 2012-11-30 at 16:24 +0000, George Dunlap wrote: > # HG changeset patch > # User George Dunlap <george.dunlap@xxxxxxxxxxxxx> > # Date 1354287957 0 > # Node ID 722da032ac90c0e1a78b1154fa588bf295d1f009 > # Parent ae6fb202b233af815466055d9f1a635802a50855 > libxl: Combine device model arg build functions > > qemu-traditional and qemu-upstream have some differences in the way > the arguments need to be passed. At the moment, this is dealt with by > having two entirely separate functions, libxl__build_device_model_args_new > and libxl__build_device_model_args_old. > > However, at least 80% of these are the same; this means that fixes or > additions to one may not make it into the other. Furthermore, there > are some unaccounable differences in implementation. FWIW: 1 file changed, 168 insertions(+), 260 deletions(-) But that doesn't really show how much of the code in the new function is shared and how much is per-qemu-version. Casting my eye over the code with the patch applied (i.e. an unscientific estimate) it seems like most of it is under an if (dm_new) of some sort. My main concern is that qemu-xen-traditional is frozen and so this code shouldn't be changing, but by bundling it with the qemu-xen code it may be subject to churn as new stuff gets added to the new qemu and plumbed through. A related concern is that some of the interfaces we use (e.g. "-hda") are deprecated in favour of more expressive forms (-drive -device et al), depending on how things go upstream we may want to switch to using them. Perhaps a useful compromise would be to pull the common stuff into a common function but call out to version specific function for the bits which differ too. Perhaps factoring into functional areas might help too? e.g. make_{disk,net,foo}_args, then the decision to combine or not can be made on a per functional area basis? > @@ -523,6 +450,9 @@ static char ** libxl__build_device_model > abort(); > } > > + if (!dm_new) > + goto finish; > + > ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb); > flexarray_append(dm_args, "-m"); > flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size)); > @@ -585,33 +515,11 @@ static char ** libxl__build_device_model > flexarray_append(dm_args, drive); > } > } > +finish: This feels especially unsatisfying... _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |