On Wed, 2016-02-17 at 12:20 +0100, Roger Pau Monné wrote: > El 16/2/16 a les 18:58, Ian Jackson ha escrit: > > Roger Pau Monne writes ("[PATCH v4 4/4] libxl: fix cd-eject"): > > > Current libxl__device_disk_set_backend implementation tried to guess > > > the > > > backend of devices with format LIBXL_DISK_FORMAT_EMPTY, which is of > > > course > > > doomed to fail since the disk is empty. Instead just return early > > > from the > > > function if an empty disk is detected. > > > > > > This fixes cd ejection. > > > > DYK when this was broken ? Or, to put it another way, how did this > > ever work ? > > > > ...looking at the code... > > > > AFAICT disk_try_backend should succeed for both LIBXL_DISK_BACKEND_PHY > > and LIBXL_DISK_BACKEND_QDISK. So even before your patch: > > > > > } > > > - memset(&a.stab, 0, sizeof(a.stab)); > > > + /* Disk is empty, so it's useless to try to guess the > > > backend type. */ > > > + return 0; > > > } else if ((disk->backend == LIBXL_DISK_BACKEND_UNKNOWN || > > > > libxl__device_disk_set_backend should work. > > I've been looking at the code and I cannot really understand how this is > supposed to work before, none of the recent changes seem to have broken > it AFAICT, and the issue has been there for a long time (~1year), which > IMHO makes no sense, so I'm quite sure I'm missing something.
cd-insert/eject seems to me to either be perpetually broken or is incredibly prone to regressing (i..e this keeps coming up). Getting a test step into osstest which used cd-insert/eject would be a good idea IMHO, either a deliberate test step which checks it works or trying to use it as a matter of course during e.g. guest install. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel