Commit Graph

278 Commits

Author SHA1 Message Date
Artom Lifshitz faa1e64e5b Fix pep8 errors with new hacking
Hacking has bumped the version of flake8 that it's using to 5.0 in its
6.0.1 release. This turns up quite a few pep8 errors lurking in our
code. Fix them.

Needed-by: https://review.opendev.org/c/openstack/hacking/+/874516
Change-Id: I3b9c7f9f5de757f818ec358c992ffb0e5f3e310f
2023-04-28 08:34:52 -04:00
Sean Mooney f3d48000b1 Add autopep8 to tox and pre-commit
autopep8 is a code formating tool that makes python code pep8
compliant without changing everything. Unlike black it will
not radically change all code and the primary change to the
existing codebase is adding a new line after class level doc strings.

This change adds a new tox autopep8 env to manually run it on your
code before you submit a patch, it also adds autopep8 to pre-commit
so if you use pre-commit it will do it for you automatically.

This change runs autopep8 in diff mode with --exit-code in the pep8
tox env so it will fail if autopep8 would modify your code if run
in in-place mode. This allows use to gate on autopep8 not modifying
patches that are submited. This will ensure authorship of patches is
maintianed.

The intent of this change is to save the large amount of time we spend
on ensuring style guidlines are followed automatically to make it
simpler for both new and old contibutors to work on nova and save
time and effort for all involved.

Change-Id: Idd618d634cc70ae8d58fab32f322e75bfabefb9d
2021-11-08 12:37:27 +00:00
Lee Yarwood 606d588e3e guestfs: With libguestfs >= v1.41.1 decode returned bytes to string
libguestfs >= v1.41.1 and commit 0ee02e0117527 changed the return type
of read_file from string to bytes.

0ee02e0117

As we don't check the version of libguestfs installed this change
handles both the original behaviour where a string is returned and the
newer behaviour by decoding any returned bytes to a string.

Closes-Bug: #1882421
Change-Id: I1c12b2903c1e5bf3a88394493456ad33233f3cd8
2021-04-23 14:02:35 +01:00
Zuul f55f5daed8 Merge "Remove VFSLocalFS" 2021-03-16 17:33:45 +00:00
Zuul bcb78e5a02 Merge "Remove non-libguestfs file injection for libvirt" 2021-03-15 11:07:45 +00:00
Balazs Gibizer 4b32f9c9e3 Remove VFSLocalFS
The fix Iac8496065c8b6212d7edac320659444ab341b513 removed the last user
of VFSLocalFS so this patch remove the class, the related tests and all
the privsep functions that become dead code after this cleanup.

Change-Id: Ia1eb1d93d1f9699a4027b7a07107109ab9a3a29a
2021-03-03 17:55:43 +01:00
Sean Dague d06a10f096 Remove non-libguestfs file injection for libvirt
This is a security concern, as mounting filesystems on the host has
had previous CVEs around executing code on the host. libguestfs is
much safer, and is the only way we should allow this.

Some caveats came up during the discussion of the bug and this change
which are documented in the release note.

Co-Authored-By: Matt Riedemann <mriedem.os@gmail.com>

Closes-Bug: #1552042

Change-Id: Iac8496065c8b6212d7edac320659444ab341b513
2021-03-03 17:54:57 +01:00
Stephen Finucane f7e3e17991 tests: Poison os.uname
We shouldn't be looking up the architecture of the test node during
tests to ensure tests work across nodes with varying architectures.

Change-Id: I458b1db091e33c0b835e44b5a86de6c0a08f99a3
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Co-authored-by: Lee Yarwood <lyarwood@redhat.com>
2021-02-20 15:32:15 +00:00
Takashi Natsume 383e2a8bdc Remove six.text_type (1/2)
Replace six.text_type with str.
A subsequent patch will replace other six.text_type.

Change-Id: I23bb9e539d08f5c6202909054c2dd49b6c7a7a0e
Implements: blueprint six-removal
Signed-off-by: Takashi Natsume <takanattie@gmail.com>
2020-12-13 11:25:31 +00:00
Stephen Finucane 21fecc7060 trivial: Remove log translations
We neither need nor want these translated so remove them. A couple of
logs with unnecessary brackets or weird indentation were identified in
the process, and these are also modified. Note that exceptions must
still be translated. Refer to [1] for more details.

[1] https://docs.openstack.org/oslo.i18n/latest/user/guidelines.html

Change-Id: I4573a7c5f3a7b5716a15fbd15ad9336807843a03
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
2020-05-27 09:40:47 +00:00
zhufl d4d7831236 [Trivial]Remove unused helper get_allocated_disk_size
Helper get_allocated_disk_size is no longer used after
d41ea9d878, this is to remove it.

Change-Id: Ib7a347b4fdb49fce893e227c72bdfbee4cea00ef
2019-08-28 13:36:52 +08:00
Sean Mooney fc9fb383c1 lxc: make use of filter python3 compatible
_detect_nbd_devices uses the filter
builtin internally to filter valid devices.

In python 2, filter returns a list. In python 3,
filter returns an iterable or generator function.
This change eagerly converts the result of calling filter
to a list to preserve the python 2 behaviour under python 3.

Closes-Bug: #1840068

Change-Id: I25616c5761ea625a15d725777ae58175651558f8
2019-08-13 22:29:57 +00:00
Zuul a8961f8f69 Merge "Fix type error on call to mount device" 2019-06-27 09:42:49 +00:00
Miguel Herranz d2ef1ce309 Fix type error on call to mount device
The call in nova.virt.disk.mount.api.Mount.mnt_dev() to
nova.privsep.fs.mount() should include the `options` argument to
fulfill with the method signature.

The test test_do_mount_need_to_specify_fs_type has been modified to
check that the caller use the correct signature.

Closes-Bug: 1829506

Change-Id: Id14993db6ea33b2da14caa4b58671fc57c182706
Signed-off-by: Miguel Herranz <miguel@midokura.com>
2019-06-26 18:16:19 -04:00
Stephen Finucane 231908a7f4 hacking: Resolve W503 (line break occurred before a binary operator)
Change-Id: I6381365ff882cf23808e8dabfce41143c5e35192
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
2019-06-24 14:24:06 -05:00
Matt Riedemann 2a8c0c800b Fix guestfs.set_backend_settings call
This method takes a list of strings. It was never failing
because python-libguestfs takes the input and casts it to
a list, which turns "force_tcg" into:

  ['f', 'o', 'r', 'c', 'e', '_', 't', 'c', 'g']

Which is obviously not correct.

http://libguestfs.org/guestfs.3.html#guestfs_set_backend_settings

This fixes that issue and leaves a TODO about investigating
the usage of set_backend_setting for just setting the
single force_tcg value instead of overrwriting all backend
settings.

Credit to Clark Boylan for identifying the issue.

Change-Id: I36edfaf30fd0cc234f929fd6937d07b6a1b646ae
Partial-Bug: #1735823
2019-05-18 18:53:13 +00:00
Stephen Finucane 3e65f778bd Bump to hacking 1.1.0
This brings in a couple of new checks which must be addressed, many of
which involve a rather large amount of changes, so these are ignored for
now. A series of follow-up changes will resolved these.

'pycodestyle' is added as a dependency rather than it being pulled in
transitively. This is necessary since we're using it in tests.

Change-Id: I35c654bd39f343417e0a1124263ff31dcd0b05c9
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
2019-04-12 16:23:49 +01:00
Michael Still fcb0691b72 Remove utils.execute() from virt.disk.api.
Change-Id: I359a412fcabe9e59c99167b35bb3be6553e5f41b
2018-12-12 09:38:20 +11:00
Lee Yarwood d41ea9d878 libvirt: Reduce calls to qemu-img during update_available_resource
I464bc2b88123a012cd12213beac4b572c3c20a56 introduced a second call to
``qemu-img`` that can easily be collapsed into one with the addition of
a new call within the disk_api.

Related-Bug: #1785827
Change-Id: Ibfd0527ed79f60282b542034d7cb97b424becba3
2018-08-08 15:22:44 +01:00
Lee Yarwood 23bd8f6263 libvirt: Report the allocated size of preallocated file based disks
At present the Libvirt driver can preallocate file based disks using the
fallocate command and importantly the `-n` option. This option allocates
blocks on the filesystem past the initial EOF of a given file:

```
$ touch test.img ; fallocate -n -l $(( 1024 * 1024 )) test.img
$ ll -lah test.img
-rw-rw-r--. 1 stack stack 0 Apr 16 13:28 test.img
$ du -h test.img
1.0M	test.img
```

This results in a miscalculation of the total disk overcommit for file
based (excluding ploop) disks as os.path.getsize is currently used to
determine the allocated size of these disks:

```
>>> import os
>>> os.path.getsize('test.img')
0
```

Using the above example the disk overcommit would be reported as 1.0M as
the disk appears empty yet will report a potential (virtual) size of 1.0M.

However as the required blocks have already been allocated on the
filesystem the host will report disk_available_least as missing an
additional 1.0M, essentially doubling the allocation for each disk.

To correct this the allocated size of file based (excluding ploop) disks
is reported using `disk_size` from the `qemu-img info` command. This
should ensure blocks allocated past the EOF of the file are taken into
account and correctly reported as allocated.

A future change should ultimately remove the use of the `-n` option with
fallocate, however as this would not help disks that have already been
allocated this has not been included in this change to simplify backports.

Change-Id: If642e51a4e186833349a8e30b04224a3687f5594
Closes-bug: #1764489
2018-04-16 20:33:42 +01:00
Michael Still 7b43fb4ebd Move xenapi disk resizing to privsep.
The same pattern as the rest of the changes. This means that privsep now
needs to let you pass flags to e2fsck, which I don't love and will remove
in a later patch.

Change-Id: I6c695c04ae586fec6adc354257638116277dda88
blueprint: hurrah-for-privsep
2018-04-13 07:09:58 +10:00
Michael Still 0751ee19d8 Move configurable mkfs to privsep.
Nova allows deployers to configure the command line which is used to create
a filesystem of a given type. This is frankly a little bit weird, but its
also historical. Move this functionality to privsep, including doing a
dance at startup to load config flags into privsep in a hopefully secure
manner.

Honestly, all of this code should be deprecated, but that's above my pay
grade and would take time to do. Oh, and maybe deployers love it the way
it is.

Change-Id: Id8eeb21e10f98a448946f178c8c5a36e48c7cac6
blueprint: hurrah-for-privsep
2018-04-04 06:29:32 +10:00
Zuul 60e6aa808d Merge "Check the return code when forcing TCG mode with libguestfs" 2018-03-12 22:59:14 +00:00
Michael Still fef1435167 Move makefs to privsep
Change-Id: I388d31d5e9c1cff10bc534ba69be899e67681ce6
blueprint: hurrah-for-privsep
2018-02-28 07:15:08 +11:00
Zuul b3f27d9d5e Merge "Don't launch guestfs in a thread pool if guestfs.debug is enabled" 2018-01-31 13:30:44 +00:00
Michael Still 29f548e67a Move flushing block devices to privsep.
A similar pattern to the others. Maybe we should write a more fully fledged
wrapper around blockdev at some point, but I think its not required yet.

Change-Id: Ie65d7c90abd0f8448500089fa48e831aef7b7abf
blueprint: hurrah-for-privsep
2017-12-13 05:22:15 +11:00
Michael Still cc33bdb239 Convert ext filesystem resizes to privsep.
This patch introduces the first code in the privsep directory which
_does_not_ run with escalated premissions. This is a requirement because
the privsep code has a restricted python path when executing. This
pattern will be used for other methods which are only sometimes
escalated.

Change-Id: Ie09e40d6476dcabda2d599e96701d419e3e8bdf0
blueprint: hurrah-for-privsep
2017-12-13 05:22:05 +11:00
Matt Riedemann 7c30da1384 Don't launch guestfs in a thread pool if guestfs.debug is enabled
When guestfs.debug is enabled, we're handling callback events
from guestfs and logging them at debug level. When guestfs
is launched to inspect capabilities, that is currently done
in an eventlet thread pool. Because of the concurrent logging
along with the eventlet thread, we can hit an issue where eventlet
tries to switch threads and fails and then we hang the launch
call to guestfs, which hangs creating an instance.

This change simply avoids using a thread pool to launch guestfs
if guestfs.debug is True.

Change-Id: I0ffe93a031154b123c8beff96a695df5a280b935
Closes-Bug: #1737214
2017-12-08 13:15:50 -05:00
Matt Riedemann ec5c27a8af Check the return code when forcing TCG mode with libguestfs
According to the guestfs docs:

http://libguestfs.org/guestfs.3.html#guestfs_set_backend_settings

This method returns a 0 on success and -1 on failure. We suspect
we might not be calling this correctly, so log a warning if it
returns a non-zero return code.

Change-Id: I14a5969667a6f6f4ae529aaf0f6b9475fdf25496
Related-Bug: #1735823
2017-12-01 15:56:34 -05:00
Michael Still 3c7a72c213 Move blkid calls to privsep.
The same pattern as before.

Change-Id: If9aaca8dd9c9a82378807bbc5d2c157e719dab4d
blueprint: hurrah-for-privsep
2017-10-26 07:16:09 +11:00
Michael Still b12f0a6026 Move kpartx calls to privsep.
The same pattern as before.

Change-Id: Ia97d7023523208f834cb088bf290b0f3c01016bc
blueprint: hurrah-for-privsep
2017-10-24 12:24:16 +00:00
Michael Still c7dae4e19b Move nbd commands to privsep.
The same pattern as previous patches. Some of these unit tests
are starting to be a bit simpler as we finish the transition.

Change-Id: If0e1fe4c0466f2f88525dc575af2ef366d4bb59d
blueprint: hurrah-for-privsep
2017-10-24 18:50:34 +11:00
Michael Still fd4b2aa4cb Move loopback setup and removal to privsep.
Once more, again.

Change-Id: I602582927c30f2929722474f68601ce47b4e98f6
blueprint: hurrah-for-privsep
2017-10-24 18:50:33 +11:00
Michael Still 7ad72b0922 Cleanup mount / umount and associated rmdir calls
Add a new filesytem mounting helper in privsep, and then start
moving things across to it. This currently implements mount and
unmount. We get to cleanup some rmdir calls while we're at it
which is nice as well.

I've added an upgrade note mentioning that we no longer ignore
the value of stderr from mount calls, as requesed in code review.

Change-Id: Ib5e585fa4bfb99617cd3ca983674114d323a3cce
blueprint: hurrah-for-privsep
2017-10-18 17:52:58 +11:00
Michael Still c1eb6f0e50 Move ploop commands to privsep.
The same pattern as the others, but with an added security concern.

Co-Authored-By: Evgeny Antyshev <eantyshev@virtuozzo.com>

Closes-Bug: #1717533

Change-Id: I1ac3a0ea4756ec68884866435c3da69171bbeb13
blueprint: hurrah-for-privsep
2017-09-28 07:29:51 +10:00
Michael Still 8ea68a5ebe Move the dac_admin privsep code to a new location.
Having utilities that are named after the context they use that no
longer exists is going to be super confusing later. Move these to
a better place.

Change-Id: Id203aa6c02c3b486f63151b3607e928990a6ca7b
blueprint: hurrah-for-privsep
2017-09-18 23:14:11 +10:00
Michael Still f535e8bb99 First attempt at adding a privsep user to nova itself.
I don't particularly care about this use case (although the localfs
code should perhaps go away), but it was a nice contained example
of a privsep user which wasn't just calling a command line.

This patch also starts to layout what an API to the privsep'd code
might look like. For now its modelled on python's os module, because
that's where all the operations we perform are coming from.

The rootwrap configuration is cleaned up as we remove users.

Co-Authored-By: Tony Breeds <tony@bakeyournoodle.com>
Change-Id: I911cc51a226d6af29d63a7a2c69253de870073e9
2017-09-07 03:01:01 +10:00
Ngo Quoc Cuong 6c3520ac5b Remove translation of log messages
The i18n team has decided not to translate the logs because it
seems like it not very useful; operators prefer to have them in
English so that they can search for those strings on the internet.

Partially fix on nova/virt other paths will be fixed on next commits

Change-Id: Ie7821aa4a5147cdb0616741bd1a1b1fc22080440
2017-06-13 11:20:28 +07:00
melissaml 5d3f0a60ce Fix a typo in documents
Removed redundant 'the'

TrivialFix

Change-Id: Ic768cee4fc2a6b3e79d8e5857e15109d27333407
2017-01-06 22:10:27 +08:00
Andrea Frittoli ce577442b0 Guestfs handle no passwd or group in image
When setting ownership of a file or directory, the guestfs driver
looks for the /etc/passwd and/or /etc/group files. In case they
are not found, the current driver lets the auges RuntimeError
through, which does not produce a very helpful error message.
Fixing that by handling the original exception and rasing a
Nova exception with more details in it.

Related-bug: #1646002

Change-Id: I2d15865c8be13b938e10e67c1b1b160f2a80f0c0
2016-12-09 12:48:44 +00:00
Kashyap Chamarthy e604946018 guestfs: Don't report exception if there's read access to kernel
Commit 92ae0f1 ("libvirt - Add log if libguestfs can't read host
kernel") reworks the logic of handling access to Kernel for libguestfs.
In doing that, it erroneously raises an exception when libguestfs is
_able_ to access the Kernel.

Fix it by reporting exception only when libguestfs does _not_ have
read access to the Kernel.

This was first tried by Kevin Zhao here
Ic6802650cb8f93e0d02c51e9014eb85a7e71f6fe, but is abandoned for some
reason.

This also adds a little more direction on what to do to fix this error.

Related-Bug: #1646002

Change-Id: Id6b4108e4e4af7c98b3e1bd9de3a09ad057e7b92
2016-12-01 15:00:39 +00:00
Takashi NATSUME 4eb89c206e Add a hacking rule for string interpolation at logging
String interpolation should be delayed to be handled
by the logging code, rather than being done
at the point of the logging call.
So add the following hacking rule for it.

- [N354] String interpolation should be delayed at logging calls.

See the oslo i18n guideline.

* http://docs.openstack.org/developer/oslo.i18n/guidelines.html

Change-Id: Ief6d3ee3539c0857098fffdb7acfeec3e0fed6eb
Closes-Bug: #1596829
2016-10-11 08:39:48 +00:00
Jenkins 015baf0d6c Merge "Don't use locals() and globals(), use a dict instead" 2016-07-25 20:51:16 +00:00
bhagyashris 046f34290d Fix invalid import order
Made corrections in import order as per OpenStack import standards [1].

[1] http://docs.openstack.org/developer/hacking/#import-order-template

TrivialFix

Change-Id: Ieb63c7fc5becc68db8d68b3e6847321e77ceeed3
2016-07-04 16:24:10 +05:30
Mikhail Feoktistov d4aa455d53 libvirt: virtuozzo instance resize support
Adapt "nova resize" code to support Virtuozzo ploop disks.
As far as ploop disks are in fact directories we add '-r' argument
to all utilities that deal with instance' disks such as cp, rsync and scp.
Thus we copy disks universally whether they are folders or files.

Also using "prl_disk_tool" instead of "qemu-img" is better for ploop images
because it resizes guest filesystem as well.

We can't resize disks from guest OS in containers,
because they are not allowed to write directly to block device.
ploop tool can resize partition table and internal filesystem,
but only for container's disks. Such disks must have only one partition
with ext filesystem.

prl_disk_tool can resize disks with internal filesystems
and doesn't require any special layout so it can resize disks
for virtual machines.  So it's better to use this tool instead of ploop.

Also we make compute.filters more strict
We call "ploop" only with "restore-descriptor" argument
And we set disk size in megabytes for prl_disk_tool

Co-Authored-By: Dmitry Guryanov <dguryanov@parallels.com>
Depends-On: I04c4379459c2fc1fd4801ec2aad53d0f6053b6d6
Change-Id: I38dbf73beb01fe1939ddca63fbfedbec1dc3c826
Implements: blueprint virtuozzo-instance-resize-support
2016-06-28 22:13:49 +03:00
Jenkins 93797372c2 Merge "libvirt - Add log if libguestfs can't read host kernel" 2016-05-09 10:53:21 +00:00
Jenkins b35f2680c4 Merge "Wait for device to be mapped" 2016-04-26 00:49:50 +00:00
Chung Chih, Hung 92ae0f1077 libvirt - Add log if libguestfs can't read host kernel
Host's kernel only allows a root user to have read/write permission in
ubuntu. If compute-service didn't have read permission then libguestfs
will launch image fail.

In libguestfs offical FAQ site had point out this issue, following is
the link
http://libguestfs.org/guestfs-faq.1.html#binaries
It had suggested users to change host's kernel permission. But this
action should be handled by other patch. Here only give a hint what's
going wrong.

Change-Id: I36c93610831e2935d46f7ee37f95619fe6dc1481
Related-Bug: 1413142
Closes-Bug: 1491216
2016-04-20 07:00:24 +00:00
Tracy Jones 4f79501215 config options: centralize section "guestfs"
The config options of the "nova.conf" section "guestfs"
got moved to the new central location "nova/conf/guestfs.py".

Change-Id: I9db186e7d665fa7ce05e806843592d739fdd9c7c
Implements: centralize-config-options-newton
2016-04-04 13:22:11 +01:00
Alexis Lee 2c1b19761b Wait for device to be mapped
There's a race condition when trying to perform file injection without
libguestfs, which causes a fallback to nbd device. Although the kpartx
command succeeds, it does so after the code has tested for success, so
Nova thinks it failed.

Retry a few times to avoid this.

Co-Authored-By: Paul Carlton <paul.carlton2@hp.com>
Change-Id: Ie5c186562475cd56c55520ad7123f47a0130b2a4
Closes-Bug: #1428639
Closes-Bug: #1484586
2016-03-16 10:34:52 +00:00