Re: [Xen-devel] [PATCH] blktap2 control function (version 2)

[eXeC001er <execooler@xxxxxxxxx>]

2010/6/28 Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>

eXeC001er writes ("[Xen-devel] [PATCH] blktap2 control function (version 2)"):
> Patch description:

Thanks for your contribution.

> -    if not os.access(disk, os.R_OK):
> -        msg = "Disk isn't accessible"
> -        log.error(msg)
> -        raise VmError(msg)
> +    attempt = 0
> +    while True:
> +        if not os.access(disk, os.R_OK) and attempt > 3:
> +            msg = "Disk isn't accessible"
> +            log.error(msg)
> +            raise VmError(msg)
> +        else:
> +            break
> +        attempt = attempt + 1

Um, can you explain why this is necessary or reasonable ?  Why three
tries ?  Why do we need to poll for this at all ?  Surely if this
helps in any particular situation it means we have a race, which
should be fixed.

(sometimes) When i use pygrub i have error: Disk isn't accessible.

> diff -r 7dcfdd45bc9e tools/python/xen/xend/XendDomainInfo.py
> --- a/tools/python/xen/xend/XendDomainInfo.py Tue Jun 22 07:31:14 2010 +0100
> +++ b/tools/python/xen/xend/XendDomainInfo.py Sun Jun 27 14:12:26 2010 +0400
> @@ -3269,7 +3269,7 @@
>                      log.info("Unmounting %s from %s." %
>                               (fn, BOOTLOADER_LOOPBACK_DEVICE))
>
> -                    dom0.destroyDevice('tap', BOOTLOADER_LOOPBACK_DEVICE)
> +                    dom0.destroyDevice(devtype, BOOTLOADER_LOOPBACK_DEVICE, force = True)

I don't understand this area of the code at all well but this change
seems plausible.

You  can see method 'destroyDevice' in class 'XendDomainInfo' in tools/python/xen/xend/XendDomainInfo.py (line 1302 +- 10 lines) (related to "bug-fix 1 and 3" in my list)

> diff -r 7dcfdd45bc9e tools/python/xen/util/blkif.py
> --- a/tools/python/xen/util/blkif.py  Tue Jun 22 07:31:14 2010 +0100
> +++ b/tools/python/xen/util/blkif.py  Sun Jun 27 14:12:26 2010 +0400
> @@ -87,7 +87,10 @@
>                  fn = "/dev/%s" %(fn,)
>
>          if typ in ("tap", "tap2"):
> -            (taptype, fn) = fn.split(":", 1)
> +            if fn.count(":") == 1:
> +                (taptype, fn) = fn.split(":", 1)
> +            else:
> +                (taptype, fn) = fn.split(":", 2)[1:3]

I'm not sure I understand this.  Is it related to this:

> 2. Bug fix for error: "File 'vhd:/path/.../disk.img' doesn't exist." (not
> correct parsing)
?

yes 

 

> 3. Bug fix for error: "Error: Device 51952 not connected" (in config file
> for DomU we should be use prefix "tap2:tapdisk:xxx" (tap2:xxx) for devices
> from (aio, ram, qcow, vhd, remus) or "tap:tapdisk:xxx" (tap:xxx) for devices
> from (sync, vmdk, qcow2, ioemu))

I haven't tried tap2 at all myself.  Is it fully supported everywhere
now ?

see above

 

Later:

> blktap2_ctrl_func.patch - for xen-4.x-testing only (because in xen-unstable
> i see previous version of patch)

Does that mean that xen-unstable needs fixing too ?  I'd rather apply
a change to xen-unstable first and test it there.

xen-unstable have my previous patch, but it can lead to regression (if use 'tap:xxx' instead  'tap:tapdisk:xxx')

 

Ian.

Thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel