<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">
<br>
<div>
<div>On Sep 6, 2013, at 14:26 , "Daniel P. Berrange" <<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>></div>
<div> wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">On Fri, Sep 06, 2013 at 10:56:10AM +0000, Alessandro Pilotti wrote:<br>
<blockquote type="cite">The RemoteFX feature allows Hyper-V compute nodes to provide GPU acceleration to instances by sharing the host's GPU resources.<br>
<br>
Blueprint: <a href="https://blueprints.launchpad.net/nova/+spec/hyper-v-remotefx">
https://blueprints.launchpad.net/nova/+spec/hyper-v-remotefx</a><br>
<br>
This feature provides big improvements for VDI related scenarios based on OpenStack and Hyper-V compute nodes.<br>
It basically impacts on the driver code only plus an additional optional scheduler filter.<br>
<br>
A full architectural description has been added in the blueprint.<br>
<br>
The patch has been published during H3 on Aug 18th and initially reviewed<br>
on Sept 4th with some very good ideas for improvements at a larger scale,<br>
subsequently implemented on the same day, unfortunately too late in the cycle.<br>
</blockquote>
<br>
Simply adding the blueprint description is not sufficient to remove<br>
my objections. The patch as proposed is too incomplete to be merged IMHO.<br>
I pointed out a number of design flaws in the review. The updated info in</blockquote>
<div>
<blockquote type="cite">the blueprint just re-inforces my review points, to further demonstrate<br>
why this patch should not be merged as it is coded today. It will require<br>
non-negliglable additional dev work to address this properly I believe,<br>
so I'm afraid that I think this is not suitable material for Havana at<br>
this point in time.<br>
</blockquote>
</div>
<div><br>
</div>
<div>I already committed an updated patch after your review on the 4th addressing your observations, agreeing that the initial implementation was missing flexibility (I only didn't commit the scheduler filter yet, as IMO it requires a separate dependent patch,
but this can be done anytime). </div>
<div><br>
</div>
<div>To allow others to add their opinion easily, I'm following up here to your objections in the blueprint which basically can be reduced (correct me if I'm wrong) to the following positions, beside the areas on which we already agreed regarding scheduling
and which have already been implemented:</div>
<div><br>
</div>
<div>1) You suggest to define the amount of RemoteFX GPU resources required in the flavour</div>
<div>2) I suggest to provide those requirements in the image custom properties (which is how it is currently implemented, see blueprint description as well). </div>
<div><br>
</div>
<div>Beside the fact that the two solutions are IMO not mutually exclusive as scheduling filters could simply apply both, the reason why I don't see how flavours should be considered for this patch is simple:</div>
<div><br>
</div>
<div>AFAIK Nova flavors at the moment don't support custom properties (please correct me if I'm wrong here, I looked at the flavors APIs and implementation, but I admit my ignorance in the flavor internals), so</div>
<div><br>
</div>
<div>1) Adding the remotefx requisites at the system metadata level <a href="https://github.com/openstack/nova/blob/master/nova/compute/flavors.py#L60">https://github.com/openstack/nova/blob/master/nova/compute/flavors.py#L60</a> would be IMO wrong, as it would
tightly couple a hypervisor specific limit with a generic compute API.</div>
<div>2) Adding custom properties to flavors goes way beyond the scope of this blueprint.</div>
<div><br>
</div>
<div>From an administrative and ACL perspective, administrators can selectively provide access to users to a given image, thus limiting the access to RemoteFX GPU resources (video memory in primis).</div>
<div><br>
</div>
<div>Being able to do it ALSO at the flavour level would add additional granularity, e.g. assigning different screen resolutions, using a single image. I personally see this as a separate blueprint as it impacts on the design of a fundamental Nova feature that
will need quite some discussion. </div>
<div><br>
</div>
<div><br>
</div>
<div>Thanks,</div>
<div><br>
</div>
<div>Alessandro</div>
<div><br>
</div>
<blockquote type="cite"><br>
Regards,<br>
Daniel<br>
-- <br>
|: <a href="http://berrange.com">http://berrange.com</a> -o- <a href="http://www.flickr.com/photos/dberrange/">http://www.flickr.com/photos/dberrange/</a> :|<br>
|: <a href="http://libvirt.org">http://libvirt.org</a> -o- <a href="http://virt-manager.org">http://virt-manager.org</a> :|<br>
|: <a href="http://autobuild.org">http://autobuild.org</a> -o- <a href="http://search.cpan.org/~danberr/">http://search.cpan.org/~danberr/</a> :|<br>
|: <a href="http://entangle-photo.org">http://entangle-photo.org</a> -o- <a href="http://live.gnome.org/gtk-vnc">http://live.gnome.org/gtk-vnc</a> :|<br>
<br>
_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev<br>
</blockquote>
</div>
<br>
</body>
</html>