Merge lp:~citrix-openstack/nova/xenapi-glance-2 into lp:~hudson-openstack/nova/trunk

[Ewan Mellor]

Implement support for streaming images from Glance when using the XenAPI virtualization backend, as per the bexar-xenapi-support-for-glance blueprint.

Images may be streamed raw, or will be streamed into the right place to allow room for an MBR and partition table, if using non-raw images. PV vs HVM detection now occurs on the image, immediately after it has been streamed. External kernel / ramdisk are also supported in this mode.

Unit test changes include a partial Glance simulator, which is stubbed in place of glance.client.Client. This allows us to pass through the VM spawn path with either glance or objectstore backends enabled; the unit tests now cover both. A dependency upon glance has been added to pip-requires, in order to pull the Glance client code into the venv.

This includes minor fixes to nova.image.glance. This code is expected to be heavily reworked anyway with the image-service-use-glance-clients work.

[Thierry Carrez (ttx)]

OK to try to get this merged, but other branches proposed before should have review priority.

[lp:~citrix-openstack/nova/xenapi-glance-2]

539. By Salvatore Orlando on 2011-01-12

Fix for _stream_disk

540. By Salvatore Orlando on 2011-01-12

Fixing the stub for _stream_disk as well

541. By Salvatore Orlando on 2011-01-12

pep8 fixes

542. By Salvatore Orlando on 2011-01-12

Fixed another issue in _stream_disk, as it did never execute _write_partition.
Fixed fake method accordingly.
Fixed pep8 errors.

[Jay Pipes (jaypipes)]

Hi!

"This includes minor fixes to nova.image.glance. This code is expected to be heavily reworked anyway with the image-service-use-glance-clients work."

The image-service-use-glance-clients is now in trunk. Does this affect this patch?

Cheers,
jay

review: Needs Information

[Thierry Carrez (ttx)]

On one hand, this branch touches relatively specific code, on the other, it seems a bit far away from being ready to be merged. I'd like to see Jay's question answered, and the review of someone familiar with the XenAPI stuff (Ozone team, anyone ?) rather than making a hasty decision on FFe.

review: Needs Information (ffe)

[Armando Migliaccio (armando-migliaccio)]

I am not entirely sure what Ewan really meant it with 'heavily reworked', but by following a discussion on the mailing list I understand that the extent of the work may be related to when glance-auth gets underway in Cactus.

As for the rest, we tried this branch internally against Glance revno 34 and everything seemed to work okay.

Hope this help!

[Sandy Walsh (sandy-walsh)]

I'm still getting up to speed on Swift/Glance/etc.

With this branch (vm_utils @514) xenapi would now have a hard dependency on glance. Is Glance ready to be a lynch-pin for Nova XenAPI deployments?

Should image store stuff be abstracted?

Just curious.

[Armando Migliaccio (armando-migliaccio)]

I always assumed that Glance was going to be the level of abstraction to the storage back-end that we needed for Nova, but as Sandy is, I am getting up to speed on this too.

[lp:~citrix-openstack/nova/xenapi-glance-2]

543. By Ewan Mellor on 2011-01-14

Merged with trunk revno 565.

[Ewan Mellor (ewanmellor)]

I don't see the image-service-use-glance-clients work in trunk. Which csets are you referring to, Jay?

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Jay Pipes
> Sent: 13 January 2011 11:13
> To: <email address hidden>
> Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> Review: Needs Information
> Hi!
>
> "This includes minor fixes to nova.image.glance. This code is expected
> to be heavily reworked anyway with the image-service-use-glance-clients
> work."
>
> The image-service-use-glance-clients is now in trunk. Does this affect
> this patch?
>
> Cheers,
> jay
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Ewan Mellor (ewanmellor)]

There is a flag (xenapi_image_service) which allows you to use nova-objectstore instead of glance, so it's not a hard dependency in that sense. There is a hard dependency on the glance client code, as you've noticed, and I didn’t see a problem with that since I don't regard glance as a "third party".

In the long run, I expect nova-objectstore to be deprecated altogether, and Glance would then be the abstraction between different image services. I don't feel the need to have an image service abstraction inside Nova, when Glance is filling that role too. If someone has a need to plug in a replacement for Glance, then we can discuss that, but I don't know of any such need.

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Sandy Walsh
> Sent: 14 January 2011 10:07
> To: <email address hidden>
> Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> I'm still getting up to speed on Swift/Glance/etc.
>
> With this branch (vm_utils @514) xenapi would now have a hard
> dependency on glance. Is Glance ready to be a lynch-pin for Nova XenAPI
> deployments?
>
> Should image store stuff be abstracted?
>
> Just curious.
>
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Ed Leafe (ed-leafe)]

Many of your localized strings are incorrect. If you have more than one formatting placeholder (e.g. %s), you must use a dictionary for your values to be substituted. The reason is that a translation may re-order the words in a phrase, and position-dependent substitution (i.e., tuples) will be ambiguous.

review: Needs Fixing

[Armando Migliaccio (armando-migliaccio)]

Hi Ed,

can you elaborate a bit more? I see lots of localized strings with multiple formatting placeholders and no use of dictionaries.

> Many of your localized strings are incorrect. If you have more than one
> formatting placeholder (e.g. %s), you must use a dictionary for your values to
> be substituted. The reason is that a translation may re-order the words in a
> phrase, and position-dependent substitution (i.e., tuples) will be ambiguous.

[Ed Leafe (ed-leafe)]

> can you elaborate a bit more? I see lots of localized strings with multiple
> formatting placeholders and no use of dictionaries.

Yes, and these will have to be corrected at some point.

The problem is that translations are not word-for-word; a phrase in different languages may order the words differently. Probably the most common example would be adjectives coming before the noun in English ("the white house"), but after the noun in others (e.g., Spanish: "la casa blanca"). If your code looks like:

color = "white"
thing = "house"
print _("The %s %s") % (color, thing)

... it will print "The white house" in English, but the Spanish will print "La blanca casa", which is wrong. You need to use a mapping for the formatting, so that the code above would read:

color = "white"
thing = "house"
print _("The %(color)s %(thing)s") % locals()

Yeah, I know that this is a weak example, since the color and thing are still in English, but it's just designed to demonstrate how positional substitution is to be avoided in localization strings.

If you run xgettext on a file that uses tuples for multiple substitutions, it will emit the following:
"warning: 'msgid' format string with unnamed arguments cannot be properly localized: The translator cannot reorder the arguments. Please consider using a format string with named arguments, and a mapping instead of a tuple for the arguments."

I hope that that helps.

[Ed Leafe (ed-leafe)]

Additional explanation at the bottom of this page: http://www.gnu.org/software/hello/manual/gettext/Python.html

[Armando Migliaccio (armando-migliaccio)]

Thanks, that explains it.

I agree with you that we need to fix this, but I personally don't see it as much of an issue as most of the log strings are made of nouns, rarely with verbs and adjectives. This makes the translation pretty straightforward.

Since it affects every log in trunk I also think it would be better to have a trunk overhaul in a separate branch.

By the way, some of the languages (i.e. Japanese) have already been translated 100%...does this mean that those translations have to be reviewed?

> > can you elaborate a bit more? I see lots of localized strings with multiple
> > formatting placeholders and no use of dictionaries.
>
> Yes, and these will have to be corrected at some point.
>
> The problem is that translations are not word-for-word; a phrase in different
> languages may order the words differently. Probably the most common example
> would be adjectives coming before the noun in English ("the white house"), but
> after the noun in others (e.g., Spanish: "la casa blanca"). If your code looks
> like:
>
> color = "white"
> thing = "house"
> print _("The %s %s") % (color, thing)
>
> ... it will print "The white house" in English, but the Spanish will print "La
> blanca casa", which is wrong. You need to use a mapping for the formatting, so
> that the code above would read:
>
> color = "white"
> thing = "house"
> print _("The %(color)s %(thing)s") % locals()
>
> Yeah, I know that this is a weak example, since the color and thing are still
> in English, but it's just designed to demonstrate how positional substitution
> is to be avoided in localization strings.
>
> If you run xgettext on a file that uses tuples for multiple substitutions, it
> will emit the following:
> "warning: 'msgid' format string with unnamed arguments cannot be properly
> localized: The translator cannot reorder the arguments. Please consider using
> a format string with named arguments, and a mapping instead of a tuple for the
> arguments."
>
> I hope that that helps.

[Jay Pipes (jaypipes)]

On Fri, Jan 14, 2011 at 12:14 PM, Armando Migliaccio
<email address hidden> wrote:
> Thanks, that explains it.
>
> I agree with you that we need to fix this, but I personally don't see it as much of an issue as most of the log strings are made of nouns, rarely with verbs and adjectives. This makes the translation pretty straightforward.
>
> Since it affects every log in trunk I also think it would be better to have a trunk overhaul in a separate branch.
>
> By the way, some of the languages (i.e. Japanese) have already been translated 100%...does this mean that those translations have to be reviewed?

I don't see this as a showstopper for this particular branch (though I
completely agree it's a bug).

We should be able to address this in Nova in Cactus as a separate
blueprint or even a bug report.

Just my 2 cents,

jay

[Jay Pipes (jaypipes)]

> I don't see the image-service-use-glance-clients work in trunk. Which csets
> are you referring to, Jay?

I was referring to this blueprint being marked completed: https://blueprints.launchpad.net/nova/+spec/image-service-use-glance-clients.

Rick, could you elaborate on the actual csets, please? Thanks!

[Rick Harris (rconradharris)]

Jay-

I was referring to https://code.launchpad.net/~rconradharris/nova/xs-snap-return-image-id-before-snapshot .

As part of that effort, I removed ParallaxClient and TellerClient in Nova-- it now uses Glance's client (glance/client.py). This is why it's failing on Hudson atm, we don't have glance packaged up and installed.

Is that what you meant, or am I misunderstanding something?

[Jay Pipes (jaypipes)]

Hi Rick,

Yeah, that's what I was referring to. OK, looks like we need to get the packaging done pronto for Glance.

/me goes off to work on that.

Cheers,
jay

[Ewan Mellor (ewanmellor)]

Yes, Rick, those are the changes we're talking about. Thanks. They're not yet in trunk, but as soon as they do go on, the xenapi-glance-2 branch will need fixing up to match.

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Rick Harris
> Sent: 14 January 2011 16:06
> To: <email address hidden>
> Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> Jay-
>
> I was referring to https://code.launchpad.net/~rconradharris/nova/xs-
> snap-return-image-id-before-snapshot .
>
> As part of that effort, I removed ParallaxClient and TellerClient in
> Nova-- it now uses Glance's client (glance/client.py). This is why
> it's failing on Hudson atm, we don't have glance packaged up and
> installed.
>
> Is that what you meant, or am I misunderstanding something?
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Ewan Mellor (ewanmellor)]

> I am not entirely sure what Ewan really meant it with 'heavily reworked', but
> by following a discussion on the mailing list I understand that the extent of
> the work may be related to when glance-auth gets underway in Cactus.

By "heavily reworked" I meant that nova/image/glance.py would be completely rewritten. This code is now in https://code.launchpad.net/~rconradharris/nova/xs-snap-return-image-id-before-snapshot, which is pending merge into trunk.

As soon as xs-snap-return-image-id-before-snapshot is merged, this will conflict with xenapi-glance-2. We will simply need to remove the changes to nova/image/glance.py, and most likely pip-requires too. There will be no additional changes on our part.

[Ewan Mellor (ewanmellor)]

I think that we've answered all the questions on this review so far:

  o I have requested a review from rconradharris, as per Thierry's request.

  o We will need to refresh this patch as soon as xs-snap-return-image-id-before-snapshot is merged. This will not be a big deal -- we'll just remove a few changes from xenapi-glance-2, in favour of the changes in xs-snap-return-image-id-before-snapshot.

  o We won't hold up this branch because of the I18N issues, but will fix those in a separate branch.

  o We don't feel the need for an abstraction between Nova and Glance, as Glance is already an abstraction across image services.

  o We have a flag that allows people to continue to use nova-objectstore, so we're not adding a hard dependency upon Glance.

  o We are adding a hard dependency upon Glance's client code (glance.client) and we think this is fine because Glance is part of OpenStack, not a third party.

With all these things addressed, please can someone review the actual content of the branch? I'm keen to get an FFE for this, and it will obviously need careful review in order to do that.

Thanks,

Ewan.

[Sandy Walsh (sandy-walsh)]

There should be some means to point to our glance installation for development purposes as a flag.

I had to add this hack to get it going:

swalsh@novadev:~/openstack/xenapi-glance-2$ bzr diff
=== modified file 'bin/nova-combined'
--- bin/nova-combined 2011-01-11 06:47:35 +0000
+++ bin/nova-combined 2011-01-15 19:43:08 +0000
@@ -34,6 +34,10 @@
 if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')):
     sys.path.insert(0, possible_topdir)

+ glance = os.path.normpath(os.path.join(possible_topdir, "..", "glance"))
+ print "added glance path:", glance
+ sys.path.insert(0, glance)
+
 gettext.install('nova', unicode=1)

Now I'm trying to figure out how to get my images in glance so I can try to create an instance. Tips welcome!

review: Needs Fixing

[Ewan Mellor (ewanmellor)]

The Glance client library is assumed to be installed, just like any other dependency. It is on PyPI, and can be easy_installed.

We _could_ add a flag to point at a source location, but that would be non-standard. Are you sure?

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Sandy Walsh
Sent: 17 January 2011 14:53
To: <email address hidden>
Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into lp:nova

Review: Needs Fixing
There should be some means to point to our glance installation for development purposes as a flag.

I had to add this hack to get it going:

swalsh@novadev:~/openstack/xenapi-glance-2$ bzr diff
=== modified file 'bin/nova-combined'
--- bin/nova-combined 2011-01-11 06:47:35 +0000
+++ bin/nova-combined 2011-01-15 19:43:08 +0000
@@ -34,6 +34,10 @@
 if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')):
     sys.path.insert(0, possible_topdir)

+ glance = os.path.normpath(os.path.join(possible_topdir, "..", "glance"))
+ print "added glance path:", glance
+ sys.path.insert(0, glance)
+
 gettext.install('nova', unicode=1)

Now I'm trying to figure out how to get my images in glance so I can try to create an instance. Tips welcome!

--
https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-2/+merge/45977
Your team Citrix OpenStack development team is subscribed to branch lp:~citrix-openstack/nova/xenapi-glance-2.

[Jay Pipes (jaypipes)]

> There should be some means to point to our glance installation for development
> purposes as a flag.
>
> I had to add this hack to get it going:
>
> swalsh@novadev:~/openstack/xenapi-glance-2$ bzr diff
> === modified file 'bin/nova-combined'
> --- bin/nova-combined 2011-01-11 06:47:35 +0000
> +++ bin/nova-combined 2011-01-15 19:43:08 +0000
> @@ -34,6 +34,10 @@
> if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')):
> sys.path.insert(0, possible_topdir)
>
> + glance = os.path.normpath(os.path.join(possible_topdir, "..", "glance"))
> + print "added glance path:", glance
> + sys.path.insert(0, glance)
> +
> gettext.install('nova', unicode=1)

As Ewan says, Glance is now packaged in PyPI, so the above shouldn't be necessary, nor would I recommend dealing with Glance differently than any other dependency.

> Now I'm trying to figure out how to get my images in glance so I can try to
> create an instance. Tips welcome!

This link should help you out :)

http://glance.openstack.org/client.html#adding-a-new-virtual-machine-image

Cheers!
jay

[Sandy Walsh (sandy-walsh)]

> The Glance client library is assumed to be installed, just like any other
> dependency. It is on PyPI, and can be easy_installed.
>
> We _could_ add a flag to point at a source location, but that would be non-
> standard. Are you sure?

Nope, certainly not sure. :) I was just thinking about situations where I'm working on bugs that span glance/nova. I prefer to run from source vs. an install so I can jump between branches.

Is there another way we can solve this problem?

[Ewan Mellor (ewanmellor)]

I just symlink into my source tree from .../site-packages/glance. I presume you could do the same thing with virtualenv too, if your dev machine didn't belong to you.

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Sandy Walsh
> Sent: 17 January 2011 17:11
> To: <email address hidden>
> Subject: Re: RE: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> > The Glance client library is assumed to be installed, just like any
> other
> > dependency. It is on PyPI, and can be easy_installed.
> >
> > We _could_ add a flag to point at a source location, but that would
> be non-
> > standard. Are you sure?
>
>
> Nope, certainly not sure. :) I was just thinking about situations where
> I'm working on bugs that span glance/nova. I prefer to run from source
> vs. an install so I can jump between branches.
>
> Is there another way we can solve this problem?
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Sandy Walsh (sandy-walsh)]

Ah, good idea ... thx. Disregard flag. :)

[Jay Pipes (jaypipes)]

OK, Ewan, Salvatore,

Rick's xs-snapshot branch is now in trunk. Can you re-merge xenapi-glance-2 with trunk and make any changes needed? Thanks much for all your patience!

[lp:~citrix-openstack/nova/xenapi-glance-2]

544. By Ewan Mellor on 2011-01-17

Merged with trunk revno 572.

[Ewan Mellor (ewanmellor)]

I have re-merged this branch with trunk, following the xs-snapshot merge. Are people otherwise happy with the code? I would like to make an FFE proposal for this if the reviewers are happy. Thanks.

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Jay Pipes
> Sent: 17 January 2011 18:22
> To: <email address hidden>
> Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> OK, Ewan, Salvatore,
>
> Rick's xs-snapshot branch is now in trunk. Can you re-merge xenapi-
> glance-2 with trunk and make any changes needed? Thanks much for all
> your patience!
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Sandy Walsh (sandy-walsh)]

I'm in the short strokes now of getting the whole thing running (using the latest branch).

1. Creating instance using objectstore
2. Switching to Glance
3. Saving snapshot. Got hung up yesterday on this.
Note: Had to convert my XenServer SR from LVM to VHD since Glance only works with VHD currently.
4. Attempting to create instance from snapshot.

Stay tuned.

(I probably won't have time to review the code as well, but I'm sure others have that covered)

[Thierry Carrez (ttx)]

Consider the exception granted if you can get this merged today !

review: Approve (ffe)

[Ewan Mellor (ewanmellor)]

Thanks Thierry!

Sandy, could you please withdraw your "Needs Fixing", as per your subsequent comments?

Ed, could you also withdraw yours? I promise we'll fix the I18N issues ASAP, but Jay would like to do that on a separate blueprint.

Jay, an Approve from you would be appreciated, if you're happy.

Rick Harris, if you're out there, a review from you (or someone else in your team) would be appreciated. We need to sneak this in while Thierry's feeling reckless ;-)

Thanks everyone,

Ewan.

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Thierry Carrez
> Sent: 18 January 2011 13:17
> To: <email address hidden>
> Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> Review: Approve ffe
> Consider the exception granted if you can get this merged today !
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Sandy Walsh (sandy-walsh)]

Hmm, think I have issues with my LVM -> VHD conversion: http://paste.openstack.org/show/488/

[Jay Pipes (jaypipes)]

On Tue, Jan 18, 2011 at 8:32 AM, Ewan Mellor <email address hidden> wrote:
> Jay, an Approve from you would be appreciated, if you're happy.

We need your help to solve a Glance packaging bug so that I can do
testing properly on the branch... but I promise this is my priority
today. If you could join IRC right quick, we can get the packaging
bug solved :) Thanks!

> Rick Harris, if you're out there, a review from you (or someone else in your team) would be appreciated.  We need to sneak this in while Thierry's feeling reckless ;-)

I'll ping Rick on IRC...

-jay

[Ed Leafe (ed-leafe)]

Decided to move correcting the localization issues into a separate blueprint.

review: Approve

[Ed Leafe (ed-leafe)]

On Jan 18, 2011, at 8:32 AM, Ewan Mellor wrote:

> Ed, could you also withdraw yours? I promise we'll fix the I18N issues ASAP, but Jay would like to do that on a separate blueprint.

 Done.

-- Ed Leafe

[Sandy Walsh (sandy-walsh)]

review: Approve

[Ewan Mellor (ewanmellor)]

It's expecting to find the local disk as per the default XenServer installation. You pesky Rackers screw with it, I think. You need SR.other_config['i18n-key'] == 'local-storage'. You can set this on the dom0 CLI using this:

sruuid=$(xe sr-list --minimal name-label="Local storage") # Or whichever SR you want to use
xe sr-param-set other-config:i18n-key=local-storage uuid=$sruuid

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Sandy Walsh
> Sent: 18 January 2011 13:32
> To: <email address hidden>
> Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> Hmm, think I have issues with my LVM -> VHD conversion:
> http://paste.openstack.org/show/488/
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Ewan Mellor (ewanmellor)]

Irritating line wrap issue there. Just to be clear, that's

sruuid=$(xe sr-list --minimal name-label="Local storage") # Or whichever SR you want to use

xe sr-param-set other-config:i18n-key=local-storage uuid=$sruuid

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Ewan Mellor
> Sent: 18 January 2011 13:40
> To: <email address hidden>
> Subject: RE: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> It's expecting to find the local disk as per the default XenServer
> installation. You pesky Rackers screw with it, I think. You need
> SR.other_config['i18n-key'] == 'local-storage'. You can set this on
> the dom0 CLI using this:
>
> sruuid=$(xe sr-list --minimal name-label="Local storage") # Or
> whichever SR you want to use
> xe sr-param-set other-config:i18n-key=local-storage uuid=$sruuid
>
> > -----Original Message-----
> > From: <email address hidden> [mailto:<email address hidden>] On Behalf
> Of
> > Sandy Walsh
> > Sent: 18 January 2011 13:32
> > To: <email address hidden>
> > Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> > lp:nova
> >
> > Hmm, think I have issues with my LVM -> VHD conversion:
> > http://paste.openstack.org/show/488/
> > --
> > https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> > 2/+merge/45977
> > Your team Citrix OpenStack development team is subscribed to branch
> > lp:~citrix-openstack/nova/xenapi-glance-2.
>
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Sandy Walsh (sandy-walsh)]

Thanks Ewan ... that worked great!

Now it gets tricky. I was hoping to use trunk to add my kernel/ramdisk/machine images to create an instance and switch over to Glance to create a snapshot, but now with the trunk merge, it always expects Glance (even though --image_service=nova.image.s3.S3ImageService is set) ... resulting in

http://paste.openstack.org/show/489/

I guess I have to hand-craft the images and pre-populate my Glance folder in order to get this to work now, huh?

Any notes on taking EC2-compliant kernel/ramdisk/machine images and prepping them for Glance usage?

[Ewan Mellor (ewanmellor)]

In the absence of any Glance CLI tools, I threw this together:

#!/usr/bin/python2.6

import sys

from glance.client import Client

hostname = sys.argv[1]
name = sys.argv[2]
typ = sys.argv[3]
filename = sys.argv[4]

c = Client(hostname, 9292)

meta = {'name': name,
        'type': typ,
        'is_public': True,
        }

with open(filename) as f:
    new_meta = c.add_image(meta, f)

print 'Stored image. Got identifier: %s' % new_meta

The third argument should be "image", "ramdisk", or "kernel". You can then use the image identifiers on the euca-run-instances command line, just as you would if you'd used euca-register.

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
> Sandy Walsh
> Sent: 18 January 2011 13:58
> To: <email address hidden>
> Subject: Re: [Merge] lp:~citrix-openstack/nova/xenapi-glance-2 into
> lp:nova
>
> Thanks Ewan ... that worked great!
>
> Now it gets tricky. I was hoping to use trunk to add my
> kernel/ramdisk/machine images to create an instance and switch over to
> Glance to create a snapshot, but now with the trunk merge, it always
> expects Glance (even though --
> image_service=nova.image.s3.S3ImageService is set) ... resulting in
>
> http://paste.openstack.org/show/489/
>
> I guess I have to hand-craft the images and pre-populate my Glance
> folder in order to get this to work now, huh?
>
> Any notes on taking EC2-compliant kernel/ramdisk/machine images and
> prepping them for Glance usage?
>
> --
> https://code.launchpad.net/~citrix-openstack/nova/xenapi-glance-
> 2/+merge/45977
> Your team Citrix OpenStack development team is subscribed to branch
> lp:~citrix-openstack/nova/xenapi-glance-2.

[Jay Pipes (jaypipes)]

On Tue, Jan 18, 2011 at 10:10 AM, Ewan Mellor <email address hidden> wrote:
> In the absence of any Glance CLI tools, I threw this together:

Coming soon to a Glance near you! :)

https://blueprints.launchpad.net/glance/+spec/admin-tool

-jay

=== added directory 'nova/tests/glance'
=== added file 'nova/tests/glance/__init__.py'
--- nova/tests/glance/__init__.py	1970-01-01 00:00:00 +0000
+++ nova/tests/glance/__init__.py	2011-01-17 19:48:42 +0000
@@ -0,0 +1,20 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+#    Copyright (c) 2011 Citrix Systems, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+:mod:`glance` -- Stubs for Glance
+=================================
+"""

=== added file 'nova/tests/glance/stubs.py'
--- nova/tests/glance/stubs.py	1970-01-01 00:00:00 +0000
+++ nova/tests/glance/stubs.py	2011-01-17 19:48:42 +0000
@@ -0,0 +1,37 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright (c) 2011 Citrix Systems, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import StringIO
+
+import glance.client
+
+
+def stubout_glance_client(stubs, cls):
+    """Stubs out glance.client.Client"""
+    stubs.Set(glance.client, 'Client',
+              lambda *args, **kwargs: cls(*args, **kwargs))
+
+
+class FakeGlance(object):
+    def __init__(self, host, port=None, use_ssl=False):
+        pass
+
+    def get_image(self, image):
+        meta = {
+            'size': 0,
+            }
+        image_file = StringIO.StringIO('')
+        return meta, image_file

=== modified file 'nova/tests/test_xenapi.py'
--- nova/tests/test_xenapi.py	2011-01-13 16:52:28 +0000
+++ nova/tests/test_xenapi.py	2011-01-17 19:48:42 +0000
@@ -34,6 +34,7 @@
 from nova.virt.xenapi.vmops import SimpleDH
 from nova.tests.db import fakes as db_fakes
 from nova.tests.xenapi import stubs
+from nova.tests.glance import stubs as glance_stubs
 
 FLAGS = flags.FLAGS
 
@@ -108,18 +109,16 @@
         conn = xenapi_conn.get_connection(False)
         volume = self._create_volume()
         instance = db.instance_create(self.values)
-        xenapi_fake.create_vm(instance.name, 'Running')
+        vm = xenapi_fake.create_vm(instance.name, 'Running')
         result = conn.attach_volume(instance.name, volume['id'], '/dev/sdc')
 
         def check():
             # check that the VM has a VBD attached to it
-            # Get XenAPI reference for the VM
-            vms = xenapi_fake.get_all('VM')
             # Get XenAPI record for VBD
             vbds = xenapi_fake.get_all('VBD')
             vbd = xenapi_fake.get_record('VBD', vbds[0])
             vm_ref = vbd['VM']
-            self.assertEqual(vm_ref, vms[0])
+            self.assertEqual(vm_ref, vm)
 
         check()
 
@@ -157,9 +156,14 @@
         FLAGS.xenapi_connection_url = 'test_url'
         FLAGS.xenapi_connection_password = 'test_pass'
         xenapi_fake.reset()
+        xenapi_fake.create_local_srs()
         db_fakes.stub_out_db_instance_api(self.stubs)
         xenapi_fake.create_network('fake', FLAGS.flat_network_bridge)
         stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
+        stubs.stubout_get_this_vm_uuid(self.stubs)
+        stubs.stubout_stream_disk(self.stubs)
+        glance_stubs.stubout_glance_client(self.stubs,
+                                           glance_stubs.FakeGlance)
         self.conn = xenapi_conn.get_connection(False)
 
     def test_list_instances_0(self):
@@ -207,40 +211,70 @@
 
         check()
 
-    def test_spawn(self):
-        instance = self._create_instance()
-
-        def check():
-            instances = self.conn.list_instances()
-            self.assertEquals(instances, [1])
-
-            # Get Nova record for VM
-            vm_info = self.conn.get_info(1)
-
-            # Get XenAPI record for VM
-            vms = xenapi_fake.get_all('VM')
-            vm = xenapi_fake.get_record('VM', vms[0])
-
-            # Check that m1.large above turned into the right thing.
-            instance_type = instance_types.INSTANCE_TYPES['m1.large']
-            mem_kib = long(instance_type['memory_mb']) << 10
-            mem_bytes = str(mem_kib << 10)
-            vcpus = instance_type['vcpus']
-            self.assertEquals(vm_info['max_mem'], mem_kib)
-            self.assertEquals(vm_info['mem'], mem_kib)
-            self.assertEquals(vm['memory_static_max'], mem_bytes)
-            self.assertEquals(vm['memory_dynamic_max'], mem_bytes)
-            self.assertEquals(vm['memory_dynamic_min'], mem_bytes)
-            self.assertEquals(vm['VCPUs_max'], str(vcpus))
-            self.assertEquals(vm['VCPUs_at_startup'], str(vcpus))
-
-            # Check that the VM is running according to Nova
-            self.assertEquals(vm_info['state'], power_state.RUNNING)
-
-            # Check that the VM is running according to XenAPI.
-            self.assertEquals(vm['power_state'], 'Running')
-
-        check()
+    def check_vm_record(self, conn):
+        instances = conn.list_instances()
+        self.assertEquals(instances, [1])
+
+        # Get Nova record for VM
+        vm_info = conn.get_info(1)
+
+        # Get XenAPI record for VM
+        vms = [rec for ref, rec
+               in xenapi_fake.get_all_records('VM').iteritems()
+               if not rec['is_control_domain']]
+        vm = vms[0]
+
+        # Check that m1.large above turned into the right thing.
+        instance_type = instance_types.INSTANCE_TYPES['m1.large']
+        mem_kib = long(instance_type['memory_mb']) << 10
+        mem_bytes = str(mem_kib << 10)
+        vcpus = instance_type['vcpus']
+        self.assertEquals(vm_info['max_mem'], mem_kib)
+        self.assertEquals(vm_info['mem'], mem_kib)
+        self.assertEquals(vm['memory_static_max'], mem_bytes)
+        self.assertEquals(vm['memory_dynamic_max'], mem_bytes)
+        self.assertEquals(vm['memory_dynamic_min'], mem_bytes)
+        self.assertEquals(vm['VCPUs_max'], str(vcpus))
+        self.assertEquals(vm['VCPUs_at_startup'], str(vcpus))
+
+        # Check that the VM is running according to Nova
+        self.assertEquals(vm_info['state'], power_state.RUNNING)
+
+        # Check that the VM is running according to XenAPI.
+        self.assertEquals(vm['power_state'], 'Running')
+
+    def _test_spawn(self, image_id, kernel_id, ramdisk_id):
+        stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
+        values = {'name': 1,
+                  'id': 1,
+                  'project_id': self.project.id,
+                  'user_id': self.user.id,
+                  'image_id': image_id,
+                  'kernel_id': kernel_id,
+                  'ramdisk_id': ramdisk_id,
+                  'instance_type': 'm1.large',
+                  'mac_address': 'aa:bb:cc:dd:ee:ff',
+                  }
+        conn = xenapi_conn.get_connection(False)
+        instance = db.instance_create(values)
+        conn.spawn(instance)
+        self.check_vm_record(conn)
+
+    def test_spawn_raw_objectstore(self):
+        FLAGS.xenapi_image_service = 'objectstore'
+        self._test_spawn(1, None, None)
+
+    def test_spawn_objectstore(self):
+        FLAGS.xenapi_image_service = 'objectstore'
+        self._test_spawn(1, 2, 3)
+
+    def test_spawn_raw_glance(self):
+        FLAGS.xenapi_image_service = 'glance'
+        self._test_spawn(1, None, None)
+
+    def test_spawn_glance(self):
+        FLAGS.xenapi_image_service = 'glance'
+        self._test_spawn(1, 2, 3)
 
     def tearDown(self):
         super(XenAPIVMTestCase, self).tearDown()

=== modified file 'nova/tests/xenapi/stubs.py'
--- nova/tests/xenapi/stubs.py	2011-01-10 09:12:48 +0000
+++ nova/tests/xenapi/stubs.py	2011-01-17 19:48:42 +0000
@@ -115,6 +115,21 @@
     stubs.Set(volume_utils, '_get_target', fake_get_target)
 
 
+def stubout_get_this_vm_uuid(stubs):
+    def f():
+        vms = [rec['uuid'] for ref, rec
+               in fake.get_all_records('VM').iteritems()
+               if rec['is_control_domain']]
+        return vms[0]
+    stubs.Set(vm_utils, 'get_this_vm_uuid', f)
+
+
+def stubout_stream_disk(stubs):
+    def f(_1, _2, _3, _4):
+        pass
+    stubs.Set(vm_utils, '_stream_disk', f)
+
+
 class FakeSessionForVMTests(fake.SessionBase):
     """ Stubs out a XenAPISession for VM tests """
     def __init__(self, uri):
@@ -124,7 +139,10 @@
         return self.xenapi.network.get_all_records()
 
     def host_call_plugin(self, _1, _2, _3, _4, _5):
-        return ''
+        sr_ref = fake.get_all('SR')[0]
+        vdi_ref = fake.create_vdi('', False, sr_ref, False)
+        vdi_rec = fake.get_record('VDI', vdi_ref)
+        return '<string>%s</string>' % vdi_rec['uuid']
 
     def VM_start(self, _1, ref, _2, _3):
         vm = fake.get_record('VM', ref)
@@ -159,10 +177,6 @@
     def __init__(self, uri):
         super(FakeSessionForVolumeTests, self).__init__(uri)
 
-    def VBD_plug(self, _1, ref):
-        rec = fake.get_record('VBD', ref)
-        rec['currently-attached'] = True
-
     def VDI_introduce(self, _1, uuid, _2, _3, _4, _5,
                       _6, _7, _8, _9, _10, _11):
         valid_vdi = False

=== modified file 'nova/virt/xenapi/fake.py'
--- nova/virt/xenapi/fake.py	2011-01-04 05:26:41 +0000
+++ nova/virt/xenapi/fake.py	2011-01-17 19:48:42 +0000
@@ -76,6 +76,7 @@
     for c in _CLASSES:
         _db_content[c] = {}
     create_host('fake')
+    create_vm('fake', 'Running', is_a_template=False, is_control_domain=True)
 
 
 def create_host(name_label):
@@ -136,14 +137,21 @@
 
 
 def create_vbd(vm_ref, vdi_ref):
-    vbd_rec = {'VM': vm_ref, 'VDI': vdi_ref}
+    vbd_rec = {
+        'VM': vm_ref,
+        'VDI': vdi_ref,
+        'currently_attached': False,
+        }
     vbd_ref = _create_object('VBD', vbd_rec)
     after_VBD_create(vbd_ref, vbd_rec)
     return vbd_ref
 
 
 def after_VBD_create(vbd_ref, vbd_rec):
-    """Create backref from VM to VBD when VBD is created"""
+    """Create read-only fields and backref from VM to VBD when VBD is
+    created."""
+    vbd_rec['currently_attached'] = False
+    vbd_rec['device'] = ''
     vm_ref = vbd_rec['VM']
     vm_rec = _db_content['VM'][vm_ref]
     vm_rec['VBDs'] = [vbd_ref]
@@ -152,9 +160,10 @@
     vbd_rec['vm_name_label'] = vm_name_label
 
 
-def create_pbd(config, sr_ref, attached):
+def create_pbd(config, host_ref, sr_ref, attached):
     return _create_object('PBD', {
         'device-config': config,
+        'host': host_ref,
         'SR': sr_ref,
         'currently-attached': attached,
         })
@@ -167,6 +176,33 @@
         })
 
 
+def create_local_srs():
+    """Create an SR that looks like the one created on the local disk by
+    default by the XenServer installer.  Do this one per host."""
+    for host_ref in _db_content['host'].keys():
+        _create_local_sr(host_ref)
+
+
+def _create_local_sr(host_ref):
+    sr_ref = _create_object('SR', {
+        'name_label': 'Local storage',
+        'type': 'lvm',
+        'content_type': 'user',
+        'shared': False,
+        'physical_size': str(1 << 30),
+        'physical_utilisation': str(0),
+        'virtual_allocation': str(0),
+        'other_config': {
+            'i18n-original-value-name_label': 'Local storage',
+            'i18n-key': 'local-storage',
+            },
+        'VDIs': []
+        })
+    pbd_ref = create_pbd('', host_ref, sr_ref, True)
+    _db_content['SR'][sr_ref]['PBDs'] = [pbd_ref]
+    return sr_ref
+
+
 def _create_object(table, obj):
     ref = str(uuid.uuid4())
     obj['uuid'] = str(uuid.uuid4())
@@ -179,9 +215,10 @@
     # Forces fake to support iscsi only
     if sr_type != 'iscsi':
         raise Failure(['SR_UNKNOWN_DRIVER', sr_type])
+    host_ref = _db_content['host'].keys()[0]
     sr_ref = _create_object(table, obj[2])
     vdi_ref = create_vdi('', False, sr_ref, False)
-    pbd_ref = create_pbd('', sr_ref, True)
+    pbd_ref = create_pbd('', host_ref, sr_ref, True)
     _db_content['SR'][sr_ref]['VDIs'] = [vdi_ref]
     _db_content['SR'][sr_ref]['PBDs'] = [pbd_ref]
     _db_content['VDI'][vdi_ref]['SR'] = sr_ref
@@ -233,6 +270,20 @@
     def __init__(self, uri):
         self._session = None
 
+    def VBD_plug(self, _1, ref):
+        rec = get_record('VBD', ref)
+        if rec['currently_attached']:
+            raise Failure(['DEVICE_ALREADY_ATTACHED', ref])
+        rec['currently_attached'] = True
+        rec['device'] = rec['userdevice']
+
+    def VBD_unplug(self, _1, ref):
+        rec = get_record('VBD', ref)
+        if not rec['currently_attached']:
+            raise Failure(['DEVICE_ALREADY_DETACHED', ref])
+        rec['currently_attached'] = False
+        rec['device'] = ''
+
     def xenapi_request(self, methodname, params):
         if methodname.startswith('login'):
             self._login(methodname, params)
@@ -289,6 +340,8 @@
             return lambda *params: self._getter(name, params)
         elif self._is_create(name):
             return lambda *params: self._create(name, params)
+        elif self._is_destroy(name):
+            return lambda *params: self._destroy(name, params)
         else:
             return None
 
@@ -299,10 +352,16 @@
                 bits[1].startswith(getter and 'get_' or 'set_'))
 
     def _is_create(self, name):
+        return self._is_method(name, 'create')
+
+    def _is_destroy(self, name):
+        return self._is_method(name, 'destroy')
+
+    def _is_method(self, name, meth):
         bits = name.split('.')
         return (len(bits) == 2 and
                 bits[0] in _CLASSES and
-                bits[1] == 'create')
+                bits[1] == meth)
 
     def _getter(self, name, params):
         self._check_session(params)
@@ -370,10 +429,9 @@
             _create_sr(cls, params) or _create_object(cls, params[1])
 
         # Call hook to provide any fixups needed (ex. creating backrefs)
-        try:
-            globals()["after_%s_create" % cls](ref, params[1])
-        except KeyError:
-            pass
+        after_hook = 'after_%s_create' % cls
+        if after_hook in globals():
+            globals()[after_hook](ref, params[1])
 
         obj = get_record(cls, ref)
 
@@ -383,6 +441,15 @@
 
         return ref
 
+    def _destroy(self, name, params):
+        self._check_session(params)
+        self._check_arg_count(params, 2)
+        table, _ = name.split('.')
+        ref = params[1]
+        if ref not in _db_content[table]:
+            raise Failure(['HANDLE_INVALID', table, ref])
+        del _db_content[table][ref]
+
     def _async(self, name, params):
         task_ref = create_task(name)
         task = _db_content['task'][task_ref]
@@ -420,7 +487,7 @@
             try:
                 return result[0]
             except IndexError:
-                return None
+                raise Failure(['UUID_INVALID', v, result, recs, k])
 
         return result
 

=== modified file 'nova/virt/xenapi/vm_utils.py'
--- nova/virt/xenapi/vm_utils.py	2011-01-12 19:47:40 +0000
+++ nova/virt/xenapi/vm_utils.py	2011-01-17 19:48:42 +0000
@@ -19,11 +19,14 @@
 their attributes like VDIs, VIFs, as well as their lookup functions.
 """
 
+import os
 import pickle
+import re
 import urllib
 from xml.dom import minidom
 
 from eventlet import event
+import glance.client
 from nova import exception
 from nova import flags
 from nova import log as logging
@@ -47,17 +50,23 @@
     'Crashed': power_state.CRASHED}
 
 
+SECTOR_SIZE = 512
+MBR_SIZE_SECTORS = 63
+MBR_SIZE_BYTES = MBR_SIZE_SECTORS * SECTOR_SIZE
+KERNEL_DIR = '/boot/guest'
+
+
 class ImageType:
-        """
-        Enumeration class for distinguishing different image types
-            0 - kernel/ramdisk image (goes on dom0's filesystem)
-            1 - disk image (local SR, partitioned by objectstore plugin)
-            2 - raw disk image (local SR, NOT partitioned by plugin)
-        """
+    """
+    Enumeration class for distinguishing different image types
+        0 - kernel/ramdisk image (goes on dom0's filesystem)
+        1 - disk image (local SR, partitioned by objectstore plugin)
+        2 - raw disk image (local SR, NOT partitioned by plugin)
+    """
 
-        KERNEL_RAMDISK = 0
-        DISK = 1
-        DISK_RAW = 2
+    KERNEL_RAMDISK = 0
+    DISK = 1
+    DISK_RAW = 2
 
 
 class VMHelper(HelperBase):
@@ -207,6 +216,25 @@
         return vif_ref
 
     @classmethod
+    def create_vdi(cls, session, sr_ref, name_label, virtual_size, read_only):
+        """Create a VDI record and returns its reference."""
+        vdi_ref = session.get_xenapi().VDI.create(
+             {'name_label': name_label,
+              'name_description': '',
+              'SR': sr_ref,
+              'virtual_size': str(virtual_size),
+              'type': 'User',
+              'sharable': False,
+              'read_only': read_only,
+              'xenstore_data': {},
+              'other_config': {},
+              'sm_config': {},
+              'tags': []})
+        LOG.debug(_('Created VDI %s (%s, %s, %s) on %s.'), vdi_ref,
+                  name_label, virtual_size, read_only, sr_ref)
+        return vdi_ref
+
+    @classmethod
     def create_snapshot(cls, session, instance_id, vm_ref, label):
         """ Creates Snapshot (Template) VM, Snapshot VBD, Snapshot VDI,
         Snapshot VHD
@@ -256,15 +284,71 @@
     def fetch_image(cls, session, instance_id, image, user, project, type):
         """
         type is interpreted as an ImageType instance
+        Related flags:
+            xenapi_image_service = ['glance', 'objectstore']
+            glance_address = 'address for glance services'
+            glance_port = 'port for glance services'
         """
+        access = AuthManager().get_access_key(user, project)
+
+        if FLAGS.xenapi_image_service == 'glance':
+            return cls._fetch_image_glance(session, instance_id, image,
+                                           access, type)
+        else:
+            return cls._fetch_image_objectstore(session, instance_id, image,
+                                                access, user.secret, type)
+
+    @classmethod
+    def _fetch_image_glance(cls, session, instance_id, image, access, type):
+        sr = find_sr(session)
+        if sr is None:
+            raise exception.NotFound('Cannot find SR to write VDI to')
+
+        c = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port)
+
+        meta, image_file = c.get_image(image)
+        virtual_size = int(meta['size'])
+        vdi_size = virtual_size
+        LOG.debug(_("Size for image %s:%d"), image, virtual_size)
+        if type == ImageType.DISK:
+            # Make room for MBR.
+            vdi_size += MBR_SIZE_BYTES
+
+        vdi = cls.create_vdi(session, sr, _('Glance image %s') % image,
+                             vdi_size, False)
+
+        with_vdi_attached_here(session, vdi, False,
+                               lambda dev:
+                               _stream_disk(dev, type,
+                                            virtual_size, image_file))
+        if (type == ImageType.KERNEL_RAMDISK):
+            #we need to invoke a plugin for copying VDI's
+            #content into proper path
+            LOG.debug(_("Copying VDI %s to /boot/guest on dom0"), vdi)
+            fn = "copy_kernel_vdi"
+            args = {}
+            args['vdi-ref'] = vdi
+            #let the plugin copy the correct number of bytes
+            args['image-size'] = str(vdi_size)
+            task = session.async_call_plugin('glance', fn, args)
+            filename = session.wait_for_task(instance_id, task)
+            #remove the VDI as it is not needed anymore
+            session.get_xenapi().VDI.destroy(vdi)
+            LOG.debug(_("Kernel/Ramdisk VDI %s destroyed"), vdi)
+            return filename
+        else:
+            return session.get_xenapi().VDI.get_uuid(vdi)
+
+    @classmethod
+    def _fetch_image_objectstore(cls, session, instance_id, image, access,
+                                 secret, type):
         url = images.image_url(image)
-        access = AuthManager().get_access_key(user, project)
         LOG.debug(_("Asking xapi to fetch %s as %s"), url, access)
         fn = (type != ImageType.KERNEL_RAMDISK) and 'get_vdi' or 'get_kernel'
         args = {}
         args['src_url'] = url
         args['username'] = access
-        args['password'] = user.secret
+        args['password'] = secret
         args['add_partition'] = 'false'
         args['raw'] = 'false'
         if type != ImageType.KERNEL_RAMDISK:
@@ -276,14 +360,21 @@
         return uuid
 
     @classmethod
-    def lookup_image(cls, session, vdi_ref):
+    def lookup_image(cls, session, instance_id, vdi_ref):
+        if FLAGS.xenapi_image_service == 'glance':
+            return cls._lookup_image_glance(session, vdi_ref)
+        else:
+            return cls._lookup_image_objectstore(session, instance_id, vdi_ref)
+
+    @classmethod
+    def _lookup_image_objectstore(cls, session, instance_id, vdi_ref):
         LOG.debug(_("Looking up vdi %s for PV kernel"), vdi_ref)
         fn = "is_vdi_pv"
         args = {}
         args['vdi-ref'] = vdi_ref
-        #TODO: Call proper function in plugin
         task = session.async_call_plugin('objectstore', fn, args)
-        pv_str = session.wait_for_task(task)
+        pv_str = session.wait_for_task(instance_id, task)
+        pv = None
         if pv_str.lower() == 'true':
             pv = True
         elif pv_str.lower() == 'false':
@@ -292,6 +383,23 @@
         return pv
 
     @classmethod
+    def _lookup_image_glance(cls, session, vdi_ref):
+        LOG.debug(_("Looking up vdi %s for PV kernel"), vdi_ref)
+
+        def is_vdi_pv(dev):
+            LOG.debug(_("Running pygrub against %s"), dev)
+            output = os.popen('pygrub -qn /dev/%s' % dev)
+            for line in output.readlines():
+                #try to find kernel string
+                m = re.search('(?<=kernel:)/.*(?:>)', line)
+                if m and m.group(0).find('xen') != -1:
+                    LOG.debug(_("Found Xen kernel %s") % m.group(0))
+                    return True
+            LOG.debug(_("No Xen kernel found.  Booting HVM."))
+            return False
+        return with_vdi_attached_here(session, vdi_ref, True, is_vdi_pv)
+
+    @classmethod
     def lookup(cls, session, i):
         """Look the instance i up, and returns it if available"""
         vms = session.get_xenapi().VM.get_by_name_label(i)
@@ -464,3 +572,123 @@
     vdi_ref = vdi_refs[0]
     vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref)
     return vdi_ref, vdi_rec
+
+
+def find_sr(session):
+    host = session.get_xenapi_host()
+    srs = session.get_xenapi().SR.get_all()
+    for sr in srs:
+        sr_rec = session.get_xenapi().SR.get_record(sr)
+        if not ('i18n-key' in sr_rec['other_config'] and
+                sr_rec['other_config']['i18n-key'] == 'local-storage'):
+            continue
+        for pbd in sr_rec['PBDs']:
+            pbd_rec = session.get_xenapi().PBD.get_record(pbd)
+            if pbd_rec['host'] == host:
+                return sr
+    return None
+
+
+def with_vdi_attached_here(session, vdi, read_only, f):
+    this_vm_ref = get_this_vm_ref(session)
+    vbd_rec = {}
+    vbd_rec['VM'] = this_vm_ref
+    vbd_rec['VDI'] = vdi
+    vbd_rec['userdevice'] = 'autodetect'
+    vbd_rec['bootable'] = False
+    vbd_rec['mode'] = read_only and 'RO' or 'RW'
+    vbd_rec['type'] = 'disk'
+    vbd_rec['unpluggable'] = True
+    vbd_rec['empty'] = False
+    vbd_rec['other_config'] = {}
+    vbd_rec['qos_algorithm_type'] = ''
+    vbd_rec['qos_algorithm_params'] = {}
+    vbd_rec['qos_supported_algorithms'] = []
+    LOG.debug(_('Creating VBD for VDI %s ... '), vdi)
+    vbd = session.get_xenapi().VBD.create(vbd_rec)
+    LOG.debug(_('Creating VBD for VDI %s done.'), vdi)
+    try:
+        LOG.debug(_('Plugging VBD %s ... '), vbd)
+        session.get_xenapi().VBD.plug(vbd)
+        LOG.debug(_('Plugging VBD %s done.'), vbd)
+        return f(session.get_xenapi().VBD.get_device(vbd))
+    finally:
+        LOG.debug(_('Destroying VBD for VDI %s ... '), vdi)
+        vbd_unplug_with_retry(session, vbd)
+        ignore_failure(session.get_xenapi().VBD.destroy, vbd)
+        LOG.debug(_('Destroying VBD for VDI %s done.'), vdi)
+
+
+def vbd_unplug_with_retry(session, vbd):
+    """Call VBD.unplug on the given VBD, with a retry if we get
+    DEVICE_DETACH_REJECTED.  For reasons which I don't understand, we're
+    seeing the device still in use, even when all processes using the device
+    should be dead."""
+    while True:
+        try:
+            session.get_xenapi().VBD.unplug(vbd)
+            LOG.debug(_('VBD.unplug successful first time.'))
+            return
+        except VMHelper.XenAPI.Failure, e:
+            if (len(e.details) > 0 and
+                e.details[0] == 'DEVICE_DETACH_REJECTED'):
+                LOG.debug(_('VBD.unplug rejected: retrying...'))
+                time.sleep(1)
+            elif (len(e.details) > 0 and
+                  e.details[0] == 'DEVICE_ALREADY_DETACHED'):
+                LOG.debug(_('VBD.unplug successful eventually.'))
+                return
+            else:
+                LOG.error(_('Ignoring XenAPI.Failure in VBD.unplug: %s'),
+                              e)
+                return
+
+
+def ignore_failure(func, *args, **kwargs):
+    try:
+        return func(*args, **kwargs)
+    except VMHelper.XenAPI.Failure, e:
+        LOG.error(_('Ignoring XenAPI.Failure %s'), e)
+        return None
+
+
+def get_this_vm_uuid():
+    with file('/sys/hypervisor/uuid') as f:
+        return f.readline().strip()
+
+
+def get_this_vm_ref(session):
+    return session.get_xenapi().VM.get_by_uuid(get_this_vm_uuid())
+
+
+def _stream_disk(dev, type, virtual_size, image_file):
+    offset = 0
+    if type == ImageType.DISK:
+        offset = MBR_SIZE_BYTES
+        _write_partition(virtual_size, dev)
+
+    with open('/dev/%s' % dev, 'wb') as f:
+        f.seek(offset)
+        for chunk in image_file:
+            f.write(chunk)
+
+
+def _write_partition(virtual_size, dev):
+    dest = '/dev/%s' % dev
+    mbr_last = MBR_SIZE_SECTORS - 1
+    primary_first = MBR_SIZE_SECTORS
+    primary_last = MBR_SIZE_SECTORS + (virtual_size / SECTOR_SIZE) - 1
+
+    LOG.debug(_('Writing partition table %d %d to %s...'),
+              primary_first, primary_last, dest)
+
+    def execute(cmd, process_input=None, check_exit_code=True):
+        return utils.execute(cmd=cmd,
+                             process_input=process_input,
+                             check_exit_code=check_exit_code)
+
+    execute('parted --script %s mklabel msdos' % dest)
+    execute('parted --script %s mkpart primary %ds %ds' %
+            (dest, primary_first, primary_last))
+
+    LOG.debug(_('Writing partition table %s done.'), dest)

=== modified file 'nova/virt/xenapi/vmops.py'
--- nova/virt/xenapi/vmops.py	2011-01-17 17:16:36 +0000
+++ nova/virt/xenapi/vmops.py	2011-01-17 19:48:42 +0000
@@ -85,7 +85,8 @@
         #Have a look at the VDI and see if it has a PV kernel
         pv_kernel = False
         if not instance.kernel_id:
-            pv_kernel = VMHelper.lookup_image(self._session, vdi_ref)
+            pv_kernel = VMHelper.lookup_image(self._session, instance.id,
+                                              vdi_ref)
         kernel = None
         if instance.kernel_id:
             kernel = VMHelper.fetch_image(self._session, instance.id,

=== modified file 'nova/virt/xenapi_conn.py'
--- nova/virt/xenapi_conn.py	2011-01-17 17:16:36 +0000
+++ nova/virt/xenapi_conn.py	2011-01-17 19:48:42 +0000
@@ -89,6 +89,9 @@
                    'The interval used for polling of remote tasks '
                    '(Async.VM.start, etc). Used only if '
                    'connection_type=xenapi.')
+flags.DEFINE_string('xenapi_image_service',
+                    'glance',
+                    'Where to get VM images: glance or objectstore.')
 flags.DEFINE_float('xenapi_vhd_coalesce_poll_interval',
                    5.0,
                    'The interval used for polling of coalescing vhds.'

=== modified file 'plugins/xenserver/xenapi/etc/xapi.d/plugins/glance'
--- plugins/xenserver/xenapi/etc/xapi.d/plugins/glance	2011-01-07 03:37:33 +0000
+++ plugins/xenserver/xenapi/etc/xapi.d/plugins/glance	2011-01-17 19:48:42 +0000
@@ -18,7 +18,7 @@
 #    under the License.
 
 #
-# XenAPI plugin for putting images into glance
+# XenAPI plugin for managing glance images
 #
 
 import base64
@@ -40,8 +40,36 @@
 configure_logging('glance')
 
 CHUNK_SIZE = 8192
+KERNEL_DIR = '/boot/guest'
 FILE_SR_PATH = '/var/run/sr-mount'
 
+def copy_kernel_vdi(session,args):
+    vdi = exists(args, 'vdi-ref')
+    size = exists(args,'image-size')
+    #Use the uuid as a filename
+    vdi_uuid=session.xenapi.VDI.get_uuid(vdi)
+    copy_args={'vdi_uuid':vdi_uuid,'vdi_size':int(size)}
+    filename=with_vdi_in_dom0(session, vdi, False,
+                              lambda dev:
+                              _copy_kernel_vdi('/dev/%s' % dev,copy_args))
+    return filename
+
+def _copy_kernel_vdi(dest,copy_args):
+    vdi_uuid=copy_args['vdi_uuid']
+    vdi_size=copy_args['vdi_size']
+    logging.debug("copying kernel/ramdisk file from %s to /boot/guest/%s",dest,vdi_uuid)
+    filename=KERNEL_DIR + '/' + vdi_uuid
+    #read data from /dev/ and write into a file on /boot/guest
+    of=open(filename,'wb')
+    f=open(dest,'rb')
+    #copy only vdi_size bytes
+    data=f.read(vdi_size)
+    of.write(data)
+    f.close()
+    of.close()    
+    logging.debug("Done. Filename: %s",filename)
+    return filename 
+
 def put_vdis(session, args):
     params = pickle.loads(exists(args, 'params'))
     vdi_uuids = params["vdi_uuids"]
@@ -128,4 +156,5 @@
 
 
 if __name__ == '__main__':
-    XenAPIPlugin.dispatch({'put_vdis': put_vdis})
+    XenAPIPlugin.dispatch({'put_vdis': put_vdis, 
+                           'copy_kernel_vdi': copy_kernel_vdi})

=== modified file 'tools/pip-requires'
--- tools/pip-requires	2011-01-13 21:18:16 +0000
+++ tools/pip-requires	2011-01-17 19:48:42 +0000
@@ -26,3 +26,4 @@
 PasteDeploy
 paste
 netaddr
+glance