Re: [openstack-dev] [nova] [libvirt] Debugging blockRebase() - "active block copy not ready for pivot"
On 10/06/2016 07:58 AM, Kashyap Chamarthy wrote: > On Thu, Oct 06, 2016 at 01:32:39AM +0200, Kashyap Chamarthy wrote: >> TL;DR >> - >> >> From the debug analysis of the log below, and discussion with Eric Blake >> of upstream QEMU / libvirt resulted in the below bug report: >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1382165 -- >> virDomainGetBlockJobInfo: Adjust job reporting based on QEMU stats & the >> "ready" field of `query-block-jobs` > > When I raised this on libvirt mailing list[0][1], one of the upstream > libvirt devs expressed an NACK in adjusting / "deliberately reporting > false data in block info structure". Similar concern was also shared by > Matt Booth on #openstack-nova IRC. I disagree with that sentiment. I think it is libvirt's responsibility to live up to libvirt's promise of virDomainGetBlockJobInfo() (namely, LIBVIRT documents that cur==end means the job is stable; and if qemu reports cur==end when the job is not stable, then it is libvirt that is lying to the upper user if it does NOT munge qemu's results to be accurate). As it is, we already patched libvirt to munge qemu's 0/0 into 0/1 when ready:false, so munging 123/123 into 122/123 when ready:false would just be another case of libvirt working around an infelicity of qemu. There is NO INHERENT MEANING to the magnitude of cur and end, nor any requirement that end stays the same value across multiple calls to virDomainGetBlockJobInfo() - they are ONLY useful for a relative comparison of how much work remains to be done. Munging the results IS appropriate. That said, if you are going to work with existing libvirt that does not munge values, then yes, you either have to implement event handling or parse XML for the ready status, as existing libvirt's virDomainGetBlockJobInfo() is insufficient for the task. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] Unvalidated user input passed to functions
On 05/15/2015 05:41 AM, Matthew Booth wrote: I was looking at the migrations api, and I noticed that the api passes the request query unchecked to get_migrations, where it ultimately ends up in a db query. I was curious and spent a couple of hours checking this morning. There are a few instances of this. I didn't find any security bugs, however I feel that this extremely bad practise, and is likely to result in a security bug eventually. For example, note that os-assisted-volume-snapshots:delete does not validate delete_info before passing it to volume_snapshot_delete. I looked at this quite carefully, and I think we are only protected from a host compromise because: 1. The api requires admin context 2. libvirt's security policy I could be wrong on that, though, so perhaps somebody else could check? Passing unvalidated input to a function isn't necessarily bad, for example if it is only used for filtering, but it should be clearly marked as such so it isn't used in an unsafe manner. This marking should follow the data as far as it goes through any number of function calls. libvirt's _volume_snapshot_delete function is a long way from the originating api call, and it is not at all obvious that the commit_base and commit_top arguments to virt_dom.blockCommit() are unvalidated. Libvirt validates that the base and top arguments to blockcommit make sense (in part because it may have to rewrite the string passed in to a different but equivalent file name for qemu to do the right thing, since qemu does strcmp rather than inode matching). Qemu also has the ability to set an arbitrary backing file string into the metadata; if this arbitrary string is under user control, then it is up to the user to validate that the string is correct to avoid breaking the chain (and doing something nasty like setting /etc/passwd as the new backing file the next time the chain is parsed from qcow2 files). But I don't think libvirt exposes the arbitrary backing name to the user, but rather computes a relative backing string itself, so that also doesn't seem to be a problem. And yes, you are protected by requiring admin context - anyone that can cause libvirt to start a new domain and write arbitrary XML already has effective root permissions on the host, because they can design the XML to hand any file of their choosing to the guest. Security is only at risk when there is elevation - if a guest could do things to cause the host to hand away privileged files, rather than only the host changing XML or backing file strings, is when we have to start worrying. The host changing strings is not elevation, just the user shooting themselves in the foot. But you are also right that it might be nice to validate strings prior to handing them to libvirt - while libvirt is able to validate that strings make sense within the chains that libvirt is aware of, it cannot know if there are additional restrictions that should be in place at the upper level (such as whether a user is entitled to access the storage locations referenced in the strings, according to nova rules). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Nova] [Cinder] [Tempest] Regarding deleting snapshot when instance is OFF
On 04/08/2015 11:22 PM, Deepak Shetty wrote: + [Cinder] and [Tempest] in the $subject since this affects them too On Thu, Apr 9, 2015 at 4:22 AM, Eric Blake ebl...@redhat.com wrote: On 04/08/2015 12:01 PM, Deepak Shetty wrote: Questions: 1) Is this a valid scenario being tested ? Some say yes, I am not sure, since the test makes sure that instance is OFF before snap is deleted and this doesn't work for fs-backed drivers as they use hyp assisted snap which needs domain to be active. Logically, it should be possible to delete snapshots when a domain is off (qemu-img can do it, but libvirt has not yet been taught how to manage it, in part because qemu-img is not as friendly as qemu in having a re-connectible Unix socket monitor for tracking long-running progress). Is there a bug/feature already opened for this ? Libvirt has this bug: https://bugzilla.redhat.com/show_bug.cgi?id=987719 which tracks generic ability of libvirt to delete snapshots; ideally, the code to manage snapshots will work for both online and persistent offline guests, but it may result in splitting the work into multiple bugs. I didn't understand much on what you mean by re-connectible unix socket :)... are you hinting that qemu-img doesn't have ability to attach to a qemu / VM process for long time over unix socket ? For online guest control, libvirt normally creates a Unix socket, then starts qemu with its -qmp monitor pointing to that socket. That way, if libvirtd goes away and then restarts, it can reconnect as a client to the existing socket file, and qemu never has to know that the person on the other end changed. With that QMP monitor, libvirt can query qemu's current state at will, get event notifications when long-running jobs have finished, and issue commands to terminate long-running jobs early, even if it is a different libvirtd issuing a later command than the one that started the command. qemu-img, on the other hand, only has the -p option or SIGUSR1 signal for outputting progress to stderr on a long-running operation (not the most machine-parseable), but is not otherwise controllable. It does not have a management connection through a Unix socket. I guess in thinking about it a bit more, a Unix socket is not essential; as long as the old libvirtd starts qemu-img in a manner that tracks its pid and collects stderr reliably, then restarting libvirtd can send SIGUSR1 to the pid and track the changes to stderr to estimate how far along things are. Also, the idea has been proposed that qemu-img is not necessary; libvirt could use qemu -M none to create a dummy machine with no CPUs and JUST disk images, and then use the qemu QMP monitor as usual to perform block operations on those disks by reusing the code it already has working for online guests. But even this approach needs coding into libvirt. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Nova] Regarding deleting snapshot when instance is OFF
On 04/08/2015 12:01 PM, Deepak Shetty wrote: Questions: 1) Is this a valid scenario being tested ? Some say yes, I am not sure, since the test makes sure that instance is OFF before snap is deleted and this doesn't work for fs-backed drivers as they use hyp assisted snap which needs domain to be active. Logically, it should be possible to delete snapshots when a domain is off (qemu-img can do it, but libvirt has not yet been taught how to manage it, in part because qemu-img is not as friendly as qemu in having a re-connectible Unix socket monitor for tracking long-running progress). 2) If this is valid scenario, then it means libvirt.py in nova should be modified NOT to raise error, but continue with the snap delete (as if volume was not attached) and take care of the dom xml (so that domain is still bootable post snap deletion), is this the way to go ? Obviously, it would be nice to get libvirt to support offline snapshot deletion, but until then, upper layers will have to work around libvirt's shortcomings. I don't know if that helps answer your questions, though. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [libvirt] [nova] The risk of hanging when shutdown instance.
On 03/30/2015 06:08 AM, Michal Privoznik wrote: On 30.03.2015 11:28, zhang bo wrote: On 2015/3/28 18:06, Rui Chen wrote: snip/ The API virDomainShutdown's description is out of date, it's not correct. In fact, virDomainShutdown would block or not, depending on its mode. If it's in mode *agent*, then it would be blocked until qemu founds that the guest actually got down. Otherwise, if it's in mode *acpi*, then it would return immediately. Thus, maybe further more work need to be done in Openstack. What's your opinions, Michal and Daniel (from libvirt.org), and Chris (from openstack.org) :) Yep, the documentation could be better in that respect. I've proposed a patch on the libvirt upstream list: https://www.redhat.com/archives/libvir-list/2015-March/msg01533.html I don't think a doc patch is right. If you don't pass any flags, then it is up to the hypervisor which method it will attempt (agent or ACPI). Yes, explicitly requesting an agent as the only method to attempt might be justifiable as a reason to block, but the overall API contract is to NOT block indefinitely. I think that rather than a doc patch, we need to fix the underlying bug, and guarantee that we return after a finite time even when the agent is involved. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] PYTHONDONTWRITEBYTECODE=true in tox.ini
On 09/15/2014 09:29 AM, Radomir Dopieralski wrote: On 12/09/14 17:11, Doug Hellmann wrote: I also use git-hooks with a post-checkout script to remove pyc files any time I change between branches, which is especially helpful if the different branches have code being moved around: git-hooks: https://github.com/icefox/git-hooks The script: $ cat ~/.git_hooks/post-checkout/remove_pyc #!/bin/sh echo Removing pyc files from `pwd` find . -name '*.pyc' | xargs rm -f exit 0 Good thing that python modules can't have spaces in their names! But for the future, find has a -delete parameter that won't break horribly on strange filenames. find . -name '*.pyc' -delete GNU find has that as an extension, but POSIX does not guarantee it, and BSD find lacks it. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] PYTHONDONTWRITEBYTECODE=true in tox.ini
On 09/15/2014 03:02 PM, Brant Knudson wrote: Good thing that python modules can't have spaces in their names! But for the future, find has a -delete parameter that won't break horribly on strange filenames. find . -name '*.pyc' -delete GNU find has that as an extension, but POSIX does not guarantee it, and BSD find lacks it. The workaround is -print0: find . -name '*.pyc' -print0 | xargs -0 rm -f Alas, both find -print0 and xargs -0 are also a GNU extensions not required by POSIX. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] PYTHONDONTWRITEBYTECODE=true in tox.ini
On 09/15/2014 03:15 PM, Eric Blake wrote:
On 09/15/2014 03:02 PM, Brant Knudson wrote:
Good thing that python modules can't have spaces in their names! But for
the future, find has a -delete parameter that won't break horribly on
strange filenames.
find . -name '*.pyc' -delete
GNU find has that as an extension, but POSIX does not guarantee it, and
BSD find lacks it.
The workaround is -print0: find . -name '*.pyc' -print0 | xargs -0 rm -f
Alas, both find -print0 and xargs -0 are also a GNU extensions not
required by POSIX.
find . -name '*.pyc' -exec rm -f \{} +
is POSIX.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
