[Qemu-devel] [PATCH v2 0/7] xen: xen-domid-restrict improvements

Ian Jackson posted 7 patches 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1507132392-25051-1-git-send-email-ian.jackson@eu.citrix.com
Test checkpatch failed
Test docker passed
Test s390x passed
There is a newer version of this series
[Qemu-devel] [PATCH v2 0/7] xen: xen-domid-restrict improvements
Posted by Ian Jackson 6 years, 6 months ago
I have been working on trying to get qemu, when running as a Xen
device model, to _actually_ not have power equivalent to root.

I think I have achieved this, with some limitations (which will be
discussed in my series against xen.git.

However, there are changes to qemu needed.  In particular

 * The -xen-domid-restrict option does not work properly right now.
   It only restricts a small subset of the descriptors qemu has open.
   I am introducing a new library call in the Xen libraries for this,
   xentoolcore_restrict_all.

 * We need to call a different function on domain shutdown.

 * The restriction operation needs to be done at a slightly different
   time, necessitating a new hook.

 * Additionally, in the future, we intend to be able to set aside
   a uid range for these qemus to run in, and that involves being
   able to tell qemu to drop privilege by numeric uid and gid.

Thanks very much to Anthony Perard for his review of the first, RFC,
version, and for helping out with configure.

At least the first patch of this, "xen: link against xentoolcore",
will very likely be necessary, since the corresponding xen.git series
is likely to make Xen 4.10.

Ian.

Re: [Qemu-devel] [PATCH v2 0/7] xen: xen-domid-restrict improvements
Posted by no-reply@patchew.org 6 years, 6 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1507132392-25051-1-git-send-email-ian.jackson@eu.citrix.com
Subject: [Qemu-devel] [PATCH v2 0/7] xen: xen-domid-restrict improvements

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
8fc7e0869d RFC configure: do_compiler: Dump some extra info under bash
549a4a2c37 os-posix: Provide new -runasid option
bc877af3d3 xen: destroy_hvm_domain: Try xendevicemodel_shutdown
b892ad5fbe xen: move xc_interface compatibility fallback further up the file
2d97091d63 xen: destroy_hvm_domain: Move reason into a variable
e0cf362a93 xen: defer call to xen_restrict until after os_setup_post
4339894b1d xen: restrict: use xentoolcore_restrict_all
77c78f244f xen: link against xentoolcore

=== OUTPUT BEGIN ===
Checking PATCH 1/8: xen: link against xentoolcore...
Checking PATCH 2/8: xen: restrict: use xentoolcore_restrict_all...
Checking PATCH 3/8: xen: defer call to xen_restrict until after os_setup_post...
Checking PATCH 4/8: xen: destroy_hvm_domain: Move reason into a variable...
Checking PATCH 5/8: xen: move xc_interface compatibility fallback further up the file...
Checking PATCH 6/8: xen: destroy_hvm_domain: Try xendevicemodel_shutdown...
Checking PATCH 7/8: os-posix: Provide new -runasid option...
ERROR: consider using qemu_strtoul in preference to strtoul
#42: FILE: os-posix.c:160:
+        lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep=='.' */

total: 1 errors, 0 warnings, 80 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 8/8: RFC configure: do_compiler: Dump some extra info under bash...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v2 0/7] xen: xen-domid-restrict improvements
Posted by Ian Jackson 6 years, 6 months ago
no-reply@patchew.org writes ("Re: [Qemu-devel] [PATCH v2 0/7] xen: xen-domid-restrict improvements"):

> ERROR: consider using qemu_strtoul in preference to strtoul
> #42: FILE: os-posix.c:160:
> +        lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep=='.' */
> 
> total: 1 errors, 0 warnings, 80 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.

As I said earlier, it is not clear to me what to do about this.

checkpatch's suggestion to use qemu_strtoul is appropriate.  But,
qemu_strtoul isn't quite a 100% replacement for all strtoul use cases,
and this is one of the exceptions.

I have added this comment and you'll see in the patch that surrounding
that line there is attention paid to the error handling, overflows,
etc.

Maybe I should submit a patch to checkpatch that causes this
suggestion to go away if qemu_strtoul is mentioned "nearby" (maybe on
the same line or an adjacent line), so that the use of qemu_strtoul
rather thanraw strtoul has indeed been "considered" as the message
suggests ?

Regards,
Ian.