Re: [libvirt] QEMU driver thread safety rules

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Wed, Oct 28, 2009 at 05:49:15PM +0000, Daniel P. Berrange wrote:
> Helper methods
> --------------
> 
> To lock the driver
> 
>   qemuDriverLock()
>     - Acquires the driver lock
> 
>   qemuDriverUnlock()
>     - Releases the driver lock
> 
> 
> 
> To lock the virDomainObjPtr
> 
>   virDomainObjLock()
>     - Acquires the virDomainObjPtr lock
> 
>   virDomainObjUnlock()
>     - Releases the virDomainObjPtr lock
> 
> 
> 
> To acquire the job condition variable (int jobActive)
> 
>   qemuDomainObjBeginJob()           (if driver is unlocked)
>     - Increments ref count on virDomainObjPtr
>     - Wait qemuDomainObjPrivate condition 'jobActive != 0' using virDomainObjPtr mutex
>     - Sets jobActive to 1
> 
>   qemuDomainObjBeginJobWithDriver() (if driver needs to be locked)
>     - Unlocks driver
>     - Increments ref count on virDomainObjPtr
>     - Wait qemuDomainObjPrivate condition 'jobActive != 0' using virDomainObjPtr mutex
>     - Sets jobActive to 1
>     - Unlocks virDomainObjPtr
>     - Locks driver
>     - Locks virDomainObjPtr
> 
>    NB: this variant is required in order to comply with lock ordering rules
>    for virDomainObjPtr vs driver
> 
> 
>   qemuDomainObjEndJob()
>     - Set jobActive to 0
>     - Signal on qemuDomainObjPrivate condition
>     - Decrements ref count on virDomainObjPtr
> 
> 
> 
> To acquire the QEMU monitor lock
> 
>   qemuDomainObjEnterMonitor()
>     - Acquires the qemuMonitorObjPtr lock
>     - Releases the virDomainObjPtr lock
> 
>   qemuDomainObjExitMonitor()
>     - Acquires the virDomainObjPtr lock
>     - Releases the qemuMonitorObjPtr lock
> 
>   NB: caller must take care to drop the driver lock if neccessary

The actual implementations of these methods I'm proposing are


    
    typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
    typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
    struct _qemuDomainObjPrivate {
        virCond jobCond; /* Use in conjunction with main virDomainObjPtr lock */
        int jobActive; /* Non-zero if a job is active. Only 1 job is allowed at any time
                        * A job includes *all* monitor commands, even those just querying
                        * information, not merely actions */
    
        qemuMonitorPtr mon;
    };
    
    static void qemuDriverLock(struct qemud_driver *driver)
    {
        virMutexLock(&driver->lock);
    }
    static void qemuDriverUnlock(struct qemud_driver *driver)
    {
        virMutexUnlock(&driver->lock);
    }
    
    
    /*
     * obj must be locked before calling, qemud_driver must NOT be locked
     *
     * This must be called by anything that will change the VM state
     * in any way, or anything that will use the QEMU monitor.
     *
     * Upon successful return, the object will have its ref count increased,
     * successful calls must be followed by EndJob eventually
     */
    static int qemuDomainObjBeginJob(virDomainObjPtr obj)
    {
        qemuDomainObjPrivatePtr priv = obj->privateData;
    
        virDomainObjRef(obj);
    
        while (priv->jobActive) {
            if (virCondWait(&priv->jobCond, &obj->lock) < 0) {
                virDomainObjUnref(obj);
                return -1;
            }
        }
        priv->jobActive = 1;
    
        return 0;
    }
    
    /*
     * obj must be locked before calling, qemud_driver must be locked
     *
     * This must be called by anything that will change the VM state
     * in any way, or anything that will use the QEMU monitor.
     */
    static int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
                                               virDomainObjPtr obj)
    {
        qemuDomainObjPrivatePtr priv = obj->privateData;
    
        virDomainObjRef(obj);
        qemuDriverUnlock(driver);
    
        while (priv->jobActive) {
            if (virCondWait(&priv->jobCond, &obj->lock) < 0)
                return -1;
        }
        priv->jobActive = 1;
    
        virDomainObjUnlock(obj);
        qemuDriverLock(driver);
        virDomainObjLock(obj);
    
        return 0;
    }
    
    /*
     * obj must be locked before calling, qemud_driver does not matter
     *
     * To be called after completing the work associated with the
     * earlier  qemuDomainBeginJob() call
     */
    static void qemuDomainObjEndJob(virDomainObjPtr obj)
    {
        qemuDomainObjPrivatePtr priv = obj->privateData;
    
        priv->jobActive = 0;
        virCondSignal(&priv->jobCond);
    
        virDomainObjUnref(obj);
    }
    
    
    /*
     * obj must be locked before calling, qemud_driver does not matter
     *
     * To be called immediately before any QEMU monitor API call
     * Must have alrady called qemuDomainObjBeginJob().
     *
     * To be followed with qemuDomainObjExitMonitor() once complete
     */
    static void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
    {
        qemuDomainObjPrivatePtr priv = obj->privateData;
    
        qemuMonitorLock(priv->mon);
        virDomainObjUnlock(obj);
    }
    
    
    /* obj must NOT be locked before calling, qemud_driver does not matter,
     * but if qemud_driver is to be locked, it should be done so before
     * this call, not after. This avoids deadlock.
     *
     * Should be paired with an earlier  qemuDomainObjEnterMonitor() call
     */
    static void qemuDomainObjExitMonitor(virDomainObjPtr obj)
    {
        qemuDomainObjPrivatePtr priv = obj->privateData;
    
        qemuMonitorUnlock(priv->mon);
        virDomainObjLock(obj);
    }
    
    
    
Daniel    
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list