On Fri, Jul 31, 2015 at 08:12:01AM +0800, zhang bo wrote:
> On 2015/7/30 17:41, zhang bo wrote:
>
> > On 2015/7/28 16:33, Ján Tomko wrote:
> >
> >> On Tue, Jul 28, 2015 at 04:25:13PM +0800, zhang bo wrote:
> >>> static int
> >>> qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
> >>> virDomainObjPtr vm,
> >>> virDomainDiskDefPtr detach)
> >>> {
> >>> .......
> >>>
> >>> rc = qemuDomainWaitForDeviceRemoval(vm);
> >>> if (rc == 0 || rc == 1)
> >>> ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
> >>> else
> >>> ret = 0; /*the return value of 2 is dismissed here, which refers
> >>> to ETIMEOUT.*/
> >>> ........
> >>> }
> >>>
> >>> ------------------------------------
> >>>
> >>> If it timeouts when qemu tries to del the device, the return value would
> >>> be modified from 2 to 0 in
> >>> function qemuDomainDetachVirtioDiskDevice(), which means that, the users
> >>> would be misleaded that
> >>> the device has been deleted, however, the device maybe probably failed to
> >>> be detached after timeout and
> >>> still in use.
> >>>
> >>
> >> This is intentional and documented:
> >> http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDetachDeviceFlags
> >>
> >> Unplugging a disk requires guest cooperation, so the best we can do is
> >> ask qemu to unplug it and wait for a while.
> >>
> >>> That is to say, the function qemuDomainDetachVirtioDiskDevice()'s return
> >>> value is ambiguous when it's 0,
> >>> maybe successful, or timeout. Will it be better to pass ETIMEOUT to user?
> >>> or any other advises? for example,
> >>> let users themselves dumpxml the guest to check whether the device has
> >>> been actually detached or not?
> >>
> >> Either dump the XML, or wait for the VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED
> >> event, as the API documentation suggests.
> >>
> >> Jan
> >
> >
> > It seems to have fixed the problem by dumping the XML or wait for the
> > DEVICE_REMOVED event. However, it seems to
> > make nova or other apps to do more checking work, they need to dump XML or
> > wait the event even if the device has
> > already been successfully removed. which is unnecessary.
> >
> > I think, it's better to return ETIMEOUT and let nova to dumpxml or wait the
> > event at this situation, rather than always
> > doing that job.
> > The app would have to listen to the event before issuing the API - otherwise the event might arrive after the client app processes the ETIMEOUT return, but before it registers the event. I think in this case it's simpler to process the event regardless of the return value. > > It maybe a better design, what's your opinion? > > > > After thinking twice, it's an async job, thus returning 0 is acceptable, > right? > Yes, that was the intention of the patch adding waiting for the event. But for the most cases, where the guest unplugs the device under 5 seconds, the API is synchronnous. Before that, the API returned 0 regardless of whether the device was unplugged or not, so this did not make matters any worse. Jan
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
