<div dir="ltr">+1<br>But we should answer the following question before switching libvirt to utils.get_image_from_system_metadata.<div><br></div><div>Some differences of utils.get_image_from_system_metadata from compute.utils.get_image_metadata are:</div><div>1) Instance metadata has a restriction of property length: long data is stripped to 255 chars.</div><div>This may cause a fault on loading serialized structure from a metadata property.</div><div><br></div><div>2) utils.get_image_from_system_metadata skips properties defined by CONF.non_inheritable_image_properties</div><div><br></div><div>So the question is: are these facts important for libvirt?</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 28, 2015 at 10:18 AM, Sahid Orentino Ferdjaoui <span dir="ltr"><<a href="mailto:sahid.ferdjaoui@redhat.com" target="_blank">sahid.ferdjaoui@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, May 27, 2015 at 02:59:10PM +0100, Daniel P. Berrange wrote:<br>
> As part of the work to object-ify the image metadata dicts, I'm looking<br>
> at the current way the libvirt driver fetches image metadata for an<br>
> instance, in cases where the compute manager hasn't already passed it<br>
> into the virt driver API. I see 2 methods that libvirt uses to get the<br>
> image metadata<br>
><br>
> - nova.utils.get_image_from_system_metadata(instance.system_metadata)<br>
><br>
> It takes the system metadata stored against the instance<br>
> and turns it into image metadata.<br>
><br>
><br>
> - nova.compute.utils.get_image_metadata(context,<br>
> image_api,<br>
> instance.image_ref,<br>
> instance)<br>
><br>
> This tries to get metadata from the image api and turns<br>
> this into system metadata<br>
><br>
> It then gets system metadata from the instance and merges<br>
> it from the data from the image<br>
><br>
> It then calls nova.utils.get_image_from_system_metadata()<br>
><br>
> IIUC, any changes against the image will override what<br>
> is stored against the instance<br>
><br>
><br>
><br>
> IIUC, when an instance is booted, the image metadata should be<br>
> saved against the instance. So I'm wondering why we need to have<br>
> code in compute.utils that merges back in the image metadata each<br>
> time ?<br>
<br>
</span>As you said during boot time we store in the instance system_metadata<br>
the image properties. Except for some special cases I don't see any<br>
reason to use the method 'get_image_metadata' and we should probably<br>
clean the code in libvirt.<br>
<span class=""><br>
> Is this intentional so that we pull in latest changes from the<br>
> image, to override what's previously saved on the instance ? If<br>
> so, then it seems that we should have been consistent in using<br>
> the compute_utils get_image_metadata() API everywhere.<br>
><br>
> It seems wrong though to pull in the latest metadata from the<br>
> image. The libvirt driver makes various decisions at boot time<br>
> about how to configure the guest based on the metadata. When we<br>
> later do changes to that guest (snapshot, hotplug, etc, etc)<br>
> we *must* use exactly the same image metadata we had at boot<br>
> time, otherwise decisions we make will be inconsistent with how<br>
> the guest is currently configured.<br>
><br>
> eg if you set hw_disk_bus=virtio at boot time, and then later<br>
> change the image to use hw_disk_bus=scsi, and then try to hotplug<br>
> a new drive on the guest, we *must* operate wrt hw_disk_bus=virtio<br>
> because the guest will not have any scsi bus present.<br>
><br>
> This says to me we should /never/ use the compute_utils<br>
> get_image_metadata() API once the guest is running, and so we<br>
> should convert libvirt to use nova.utils.get_image_from_system_metadata()<br>
> exclusively.<br>
><br>
> It also makes me wonder how nova/compute/manager.py is obtaining image<br>
> meta in cases where it passes it into the API and whether that needs<br>
> changing at all.<br>
<br>
</span>+1<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> Regards,<br>
> Daniel<br>
> --<br>
> |: <a href="http://berrange.com" target="_blank">http://berrange.com</a> -o- <a href="http://www.flickr.com/photos/dberrange/" target="_blank">http://www.flickr.com/photos/dberrange/</a> :|<br>
> |: <a href="http://libvirt.org" target="_blank">http://libvirt.org</a> -o- <a href="http://virt-manager.org" target="_blank">http://virt-manager.org</a> :|<br>
> |: <a href="http://autobuild.org" target="_blank">http://autobuild.org</a> -o- <a href="http://search.cpan.org/~danberr/" target="_blank">http://search.cpan.org/~danberr/</a> :|<br>
> |: <a href="http://entangle-photo.org" target="_blank">http://entangle-photo.org</a> -o- <a href="http://live.gnome.org/gtk-vnc" target="_blank">http://live.gnome.org/gtk-vnc</a> :|<br>
><br>
> __________________________________________________________________________<br>
> OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div>