It seems rather complicated, but I can't think of a much better way to deal
with it, so just a handful of minor nit-picks inline...
On Thu, Jun 12, 2008 at 12:23:06PM +0100, Ian Jackson wrote:
> diff --exclude=.hg --exclude=.config --exclude='*.old' --exclude='*.orig'
> --exclude='*~' -ruN ../xen-unstable-3.hg/tools/python/xen/xend/XendDomain.py
> tools/python/xen/xend/XendDomain.py
> --- ../xen-unstable-3.hg/tools/python/xen/xend/XendDomain.py 2008-06-12
> 12:13:04.000000000 +0100
> +++ tools/python/xen/xend/XendDomain.py 2008-06-11 16:42:07.000000000
> +0100
> @@ -34,7 +34,7 @@
>
> from xen.xend import XendOptions, XendCheckpoint, XendDomainInfo
> from xen.xend.PrettyPrint import prettyprint
> -from xen.xend import XendConfig
> +from xen.xend import XendConfig, image
> from xen.xend.XendError import XendError, XendInvalidDomain, VmError
> from xen.xend.XendError import VMBadState
> from xen.xend.XendLogging import log
> @@ -179,6 +179,8 @@
> log.exception("Failed to create reference to running
> "
> "domain id: %d" % dom['domid'])
>
> + image.cleanup_stale_sentinel_fifos()
> +
> # add all managed domains as dormant domains.
> for dom in managed:
This indentation looks wrong to me.
> diff --exclude=.hg --exclude=.config --exclude='*.old' --exclude='*.orig'
> --exclude='*~' -ruN
> ../xen-unstable-3.hg/tools/python/xen/xend/XendDomainInfo.py
> tools/python/xen/xend/XendDomainInfo.py
> --- ../xen-unstable-3.hg/tools/python/xen/xend/XendDomainInfo.py
> 2008-06-12 12:13:04.000000000 +0100
> +++ tools/python/xen/xend/XendDomainInfo.py 2008-06-11 15:02:43.000000000
> +0100
> @@ -30,6 +30,7 @@
> import re
> import copy
> import os
> +import fcntl
> import traceback
> from types import StringTypes
>
This this change intended ? You're not adding any calls to fcntl in this patch.
> diff --exclude=.hg --exclude=.config --exclude='*.old' --exclude='*.orig'
> --exclude='*~' -ruN ../xen-unstable-3.hg/tools/python/xen/xend/image.py
> tools/python/xen/xend/image.py
> --- ../xen-unstable-3.hg/tools/python/xen/xend/image.py 2008-06-12
> 12:13:04.000000000 +0100
> +++ tools/python/xen/xend/image.py 2008-06-12 12:16:14.000000000 +0100
>
>
> null = os.open("/dev/null", os.O_RDONLY)
> - logfd = os.open(logfile, os.O_WRONLY|os.O_CREAT|os.O_TRUNC)
> -
> + logfd = os.open(self.logfile,
> os.O_WRONLY|os.O_CREAT|os.O_TRUNC|os.O_APPEND)
> +
Don't need to add O_APPEND, since we're truncating the file.
> @@ -356,18 +389,26 @@
> os.dup2(logfd, 2)
> os.close(null)
> os.close(logfd)
> + self.sentinel_fifo.close()
> try:
> os.execve(self.device_model, args, env)
> - except:
> - os._exit(127)
> + except Exception, e:
> + print >>sys.stderr, (
> + 'failed to set up fds or execute dm %s: %s' %
> + (self.device_model, utils.exception_string(e)))
> + os._exit(126)
> except:
> os._exit(127)
> else:
> self.pid = pid
> os.close(null)
> os.close(logfd)
> + sentinel_write.close()
> self.vm.storeDom("image/device-model-pid", self.pid)
> log.info("device model pid: %d", self.pid)
Broken indentation here.
> def destroyDeviceModel(self):
> if self.device_model is None:
> return
> + self
Typo, I guess ?
Regards,
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 :|
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home