* Chris Lalancette (clala...@redhat.com) wrote:
> The code in libvirt that does ACS checking has a pretty
> serious bug that was essentially rendering the check useless.
> When trying to assign a device, we have to check that all
> bridges upstream of this device support ACS. That means
> that we have to find the parent of the current device,
> check for ACS, then find the parent of that device, check
> for ACS, etc.
>
> However, the code to find the parent of the device had a
> much too relaxed check. It would iterate through all PCI
> devices on the system, looking for a device that had a range
> of busses that included the current device's bus.
>
> That's not correct, though. Depending on how we iterated
> through the list of PCI devices, we could first find the
> *topmost* bridge in the system; since it necessarily had
> a range of busses including the current device's bus, we
> would only ever check the topmost bridge, and not check
> any of the intermediate bridges.
>
> This patch is simple in that it looks for the PCI device
> whose secondary device *exactly* equals the bus of the
> device we are looking for. That means that one, and only one
> bridge will be found, and it will be the correct device.
>
> Note that this also caused a fairly serious bug in the
> secondary bus reset code, where we could erroneously
> reset the topmost bus instead of the inner bus.
>
> This patch was tested by me on a 4-port NIC with a
> bridge without ACS (where assignment failed), a 4-port
> NIC with a bridge with ACS (where assignment succeeded),
> and a 2-port NIC with no bridges (where assignment
> succeeded).
Hmm, I'm not sure this will work with all SR-IOV devices. VFs can
exist on a bus number that is >= than the PF bus number. When it's > PF,
the upstream switch effectively addresses it via the subordinate value
(not secondary).
Can this be loosened to look for the "leaf" parent? I.e. one where the
secondary bus matches, or the secondary < bus <= subordinate, and no
other bridge has the subordinate value as the secondary value.
thanks,
-chris
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Rawhide's gcc 4.5.0 got smarter in relation to Fedora 13's gcc 4.4.4,
and exposed a real bug (introduced by commit ed9c14a7 this January)
as well as extra noise that is easy enough to silence.
The new warnings looked like:
esx/esx_vi_types.c:1231:1: warning: logical 'or' of collectively exhaustive
tests is always true [-Wlogical-op]
And the esx_vi_types.c case is proof why it is a bad idea to ever
stick -Werror in a spec file - upgrade the compiler, and something
that previously built silently can now fail to build; in contrast,
using -Werror during development is great as it forces you to think
about the issue, and in the case of xen, fix a real bug.
Eric Blake (2):
xen: fix logic bug
esx: silence spurious compiler warning
src/esx/esx_vi_types.c |6 --
src/xen/xend_internal.c |6 +++---
2 files changed, 7 insertions(+), 5 deletions(-)
--
1.7.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
moveInterfaceToNetNs and vethInterfaceUpOrDown may return a
positive value when they fail. We should check if the value
is not zero instead of checking if it's negative.
This defect may be related to the bug:
https://bugzilla.redhat.com/show_bug.cgi?id=607496 .
It would not fix the bug, but would unveil what happens.
And also, the return value of positive is an exit code, not
an errno. So we should not use virReportSystemError.
Note that there still remains a problem that the descriptions
of the functions are wrong; they say that the return value
will be -1 on failure, however, they would actually return a
positive value. The inconsistent should be fixed at some point.
---
src/lxc/lxc_container.c |7 ++-
src/lxc/lxc_controller.c |2 +-
src/lxc/lxc_driver.c | 12 ++--
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4371dba..c77d262 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -273,8 +273,13 @@ static int lxcContainerRenameAndEnableInterfaces(unsigned
int nveths,
}
/* enable lo device only if there were other net devices */
-if (veths)
+if (veths) {
rc = vethInterfaceUpOrDown("lo", 1);
+if (0 != rc) {
+VIR_ERROR(_("Failed to enable lo (%d)"), rc);
+rc = -1;
+}
+}
error_out:
VIR_FREE(newname);
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index d8b7bc7..9829a69 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -477,7 +477,7 @@ static int lxcControllerMoveInterfaces(unsigned int nveths,
{
unsigned int i;
for (i = 0 ; i < nveths ; i++)
-if (moveInterfaceToNetNs(veths[i], container) < 0) {
+if (moveInterfaceToNetNs(veths[i], container) != 0) {
lxcError(VIR_ERR_INTERNAL_ERROR,
_("Failed to move interface %s to ns %d"),
veths[i], container);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 4fc1ecd..51c273e 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -886,9 +886,9 @@ static int lxcSetupInterfaces(virConnectPtr conn,
char macaddr[VIR_MAC_STRING_BUFLEN];
virFormatMacAddr(def->nets[i]->mac, macaddr);
if (0 != (rc = setMacAddr(containerVeth, macaddr))) {
-virReportSystemError(rc,
- _("Failed to set %s to %s"),
- macaddr, containerVeth);
+lxcError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to set %s to %s (%d)"),
+ macaddr, containerVeth, rc);
goto error_exit;
}
}
@@ -901,9 +901,9 @@ static int lxcSetupInterfaces(virConnectPtr conn,
}
if (0 != (rc = vethInterfaceUpOrDown(parentVeth, 1))) {
-virReportSystemError(rc,
- _("Failed to enable %s device"),
- parentVeth);
+lxcError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to enable %s device (%d)"),
+ parentVeth, rc);
goto error_exit;
}
--
1.6.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 28, 2010 at 10:15 AM, Laine Stump wrote:
> On 07/27/2010 11:11 AM, Ryota Ozaki wrote:
>>
>> On Mon, Jul 26, 2010 at 11:37 PM, Laine Stump wrote:
>>>
>>> On 07/26/2010 08:31 AM, Ryota Ozaki wrote:
On Mon, Jul 26, 2010 at 6:51 PM, Daniel P. Berrange
wrote:
>
> You could just change
>
> return cmdResult
>
> to
>
> return -cmdResult;
>
> That would still let you give the error code, while also keeping the
> value
> < 0
It looks better than mine ;-) I'll rewrite my patch in such a way.
Laine, is it ok for you too?
>>> Doing that is fine when all possible failures in the function have an
>>> associated errno. In the case of virRun'ing an external program that
>>> could
>>> return a non-zero exit code, this is unfortunately not the case, so those
>>> functions that call virRun will need to report the error themselves and
>>> return -1.
>>
>> For veth.c all functions match the latter case while bridge.c has both.
>> If we don't take care about the consistency between veth.c and bridge.c,
>> we can focus on how to treat the latter case. (Ideally keeping the
>> consistency
>> is better, but it seems difficult...)
>
> We can ignore bridge.c for now - there is a lot of code in libvirt that
> doesn't follow the "-errno on failure" convention, so we should try to be as
> narrow as possible. For this cleanup, I like the scope of just veth.c and
> its direct callers.
OK.
>
> Another thing I didn't notice when I wrote my first comment on this thread
> is that most of the error conditions in the involved functions are due to an
> external program failing, which will likely be due to the program returning
> a non-0 code. However, that's not *always* the case, so we can't really say
> that the function will always return a valid "-exitcode", nor can we say it
> will always return a valid "-errno" (and as you point out, they can
> conflict).
I'm not sure the valid "-exitcode", do you mean that virRun itself may fail
and return non-zero (not cmdResult)?
>
>
>>> When non-0 exits from the called program are all failures, the simplest
>>> way
>>> to do it is, as you say, to just not pass a pointer to a resultcode to
>>> virRun (as long as the error message reported by virRun is useful enough
>>> -
>>
>> Yes.
>>
>>> remember that it gives the name of the program being run, and "virRun",
>>> but
>>> not the name of the calling function).
>>
>> Agreed. That'll lose useful information for debugging. One option is
>> to re-report
>> an error in the calling function. It will lead reporting two messages,
>> but it should
>> be more useful than less reports.
>>
>> One concern aobut virRun's error reporting is that it shows standard
>> errors through
>> DEBUG so by default it shows nothing while they are important for ifconfig
>> and ip commands because their error messages may be according to errno.
>
> I also recall once in some other code letting virRun display the error and
> the result was that someone else who got the error (which was really
> unimportant) was handed a giant scary looking (and uninformative) error
> message that took their attention away from the real problem, which was
> elsewhere (I forget the details now, but you get the idea). It's convenient,
> but if you want meaningful errors, I would recommend against letting virRun
> report them.
I agree virRun's error message isn't suit for displaying to users, but it's for
debugging. What I want to say here is that I want to log the message
somewhere. (I'm sorry I didn't distinguish well between error reporting to
users and logging to a log file.)
BTW, lxc driver has a problem that lxc controller process cannot transfer
any errors to libvirtd (and to virsh). Errors which happen in the controller
may be lost. At this point, logging is the only way to store error messages.
(Am I wrong?)
(Even worse, libvirtd would return control back to virsh before a controller
surely executes init process of lxc. This defect leads a situation that virsh
exists successfully, but the container is actually not running. I thinks
we would have to fix those issues at some point.)
>
> I went through all the calls to all the functions listed in veth.h, and I
> see that they are called from relatively few places, and the error logs are
> almost always to the veth.c function that's being called, not to the caller.
> So I think it could work to have:
>
> 1) all calls to virRun in veth.c request cmdResult.
>
> 2) all functions in veth.c log the errors themselves (with one possible
> exception - vethDelete() - because it is often called during error
> recovery).
>
> 3) all functions in veth.c return -1 to their caller
>
> 4) all functions called by those functions also return -1 (this is mostly
> the case already, once you change veth.c, and has no effect on those
> functions' callers).
>
> Logging errors in veth.c functions you have all the information you need
> from virRun, but als
On 07/28/2010 03:07 PM, Chris Lalancette wrote:
> However, the code to find the parent of the device had a
> much too relaxed check. It would iterate through all PCI
> devices on the system, looking for a device that had a range
> of busses that included the current device's bus.
Stupid English. I'm more used seeing buses than busses, and was about
to call you on it, but
http://www.googlefight.com/index.php?lang=en_GB&word1=buses&word2=busses
shows both forms as roughly neck-and-neck and dictionary.com lists both
spellings as acceptable plurals.
> This patch is simple in that it looks for the PCI device
> whose secondary device *exactly* equals the bus of the
> device we are looking for. That means that one, and only one
> bridge will be found, and it will be the correct device.
Makes sense.
>
> Note that this also caused a fairly serious bug in the
s/caused/solved/ - the bug was caused by the condition before this
commit, but the commit message is documenting what this particular
commit does for the code base.
>
> /* No, it's superman! */
> -return (dev->bus >= secondary && dev->bus <= subordinate);
> +return (dev->bus == secondary);
Do we want a comment here in the code to summarize your commit message?
But that's a minor nit, so:
ACK.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
* Chris Lalancette (clala...@redhat.com) wrote:
> If detecting the FLR flag of a pci device fails, then we
> could run into the situation of trying to close a file
> descriptor twice, once in pciInitDevice() and once in pciFreeDevice().
> Fix that by removing the pciCloseConfig() in pciInitDevice() and
> just letting pciFreeDevice() handle it.
>
> Thanks to Chris Wright for pointing out this problem.
>
> While we are at it, fix an error check. While it would actually
> work as-is (since success returns 0), it's still more clear to
> check for < 0 (as the rest of the code does).
>
> Signed-off-by: Chris Lalancette
Acked-by: Chris Wright
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
'./autobuild.sh' with lcov installed discovered that our
coverage support has been bit-rotting for a while. This
restores it back to a successful state, although I have
not yet spent any time looking through the resulting files to
look for low-hanging fruit in the unit test coverage front.
* configure.ac: Clear COMPILER_FLAGS at right place.
* Makefile.am (cov): Newer genhtml no longer likes plain -s.
* m4/compiler-flags.m4 (gl_COMPILER_FLAGS): Don't AC_SUBST
COMPILER_FLAGS; it is a shell variable for use in configure only.
* src/Makefile.am (AM_CFLAGS, AM_LDFLAGS): New variables, to make
it easier to provide global flag additions. Use throughout, to
uniformly apply coverage flags.
* .gitignore: Globally ignore gcov output.
* daemon/.gitignore: Simplify.
* src/.gitignore: Likewise.
* tests/.gitignore: Likewise.
---
v3 had an ACK at:
https://www.redhat.com/archives/libvir-list/2010-June/msg00660.html
but I never applied it because it was part of a larger series that
had not been completed.
I'm now at the point where this is easy enough to include in
0.8.3, while leaving the docs VPATH fixups for post release, but it
has changed enough to need another ACK before I can push.
Changes from v3:
rebase to present status (several conflicts resolved in src/Makefile.am)
fix 'cov' rule in Makefile.am to allow VPATH usage
.gitignore |3 +
Makefile.am | 10 +++-
configure.ac |1 +
daemon/.gitignore|2 -
m4/compiler-flags.m4 |1 -
src/.gitignore |3 -
src/Makefile.am | 112 +++---
tests/.gitignore |2 -
8 files changed, 71 insertions(+), 63 deletions(-)
diff --git a/.gitignore b/.gitignore
index 39ed671..c62e885 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,9 @@
*.#*#
*.a
*.exe
+*.gcda
+*.gcno
+*.gcov
*.o
*.orig
*.rej
diff --git a/Makefile.am b/Makefile.am
index a6af20f..c5d278b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -68,10 +68,14 @@ tests:
cov: clean-cov
mkdir $(top_builddir)/coverage
- $(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp -d
$(top_srcdir)/src -d $(top_srcdir)/daemon -d $(top_srcdir)/tests
- $(LCOV) -r $(top_builddir)/coverage/libvirt.info.tmp -o
$(top_builddir)/coverage/libvirt.info *usr*
+ $(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \
+ -d $(top_builddir)/src -d $(top_builddir)/daemon \
+ -d $(top_builddir)/tests
+ $(LCOV) -r $(top_builddir)/coverage/libvirt.info.tmp \
+ -o $(top_builddir)/coverage/libvirt.info
rm $(top_builddir)/coverage/libvirt.info.tmp
- $(GENHTML) -s -t "libvirt" -o $(top_builddir)/coverage --legend
$(top_builddir)/coverage/libvirt.info
+ $(GENHTML) --show-details -t "libvirt" -o $(top_builddir)/coverage \
+ --legend $(top_builddir)/coverage/libvirt.info
clean-cov:
rm -rf $(top_builddir)/coverage
diff --git a/configure.ac b/configure.ac
index 7bc0e79..70432c1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1752,6 +1752,7 @@ AC_ARG_ENABLE([test-coverage],
enable_coverage=$enableval
if test "${enable_coverage}" = yes; then
+ COMPILER_FLAGS=
gl_COMPILER_FLAGS(-fprofile-arcs)
gl_COMPILER_FLAGS(-ftest-coverage)
AC_SUBST([COVERAGE_CFLAGS], [$COMPILER_FLAGS])
diff --git a/daemon/.gitignore b/daemon/.gitignore
index 4e6123e..4475b6d 100644
--- a/daemon/.gitignore
+++ b/daemon/.gitignore
@@ -1,5 +1,3 @@
-*.gcda
-*.gcno
*.la
*.lo
.deps
diff --git a/m4/compiler-flags.m4 b/m4/compiler-flags.m4
index 628bd1f..6db4816 100644
--- a/m4/compiler-flags.m4
+++ b/m4/compiler-flags.m4
@@ -23,7 +23,6 @@
AC_DEFUN([gl_COMPILER_FLAGS],
[AC_MSG_CHECKING(whether compiler accepts $1)
- AC_SUBST(COMPILER_FLAGS)
ac_save_CFLAGS="$CFLAGS"
dnl Some flags are dependant, so we set all previously checked
dnl flags when testing. Except for -Werror which we have to
diff --git a/src/.gitignore b/src/.gitignore
index d24a87d..5d114c9 100644
--- a/src/.gitignore
+++ b/src/.gitignore
@@ -6,9 +6,6 @@ Makefile.in
*.loT
*.la
*.exe
-*.gcda
-*.gcno
-*.gcov
*.cov
libvirt_parthelper
libvirt_lxc
diff --git a/src/Makefile.am b/src/Makefile.am
index f71f72c..940bd33 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -24,6 +24,9 @@ INCLUDES =
\
-DIN_LIBVIRT\
$(WIN32_EXTRA_CFLAGS)
+AM_CFLAGS = $(COVERAGE_CFLAGS)
+AM_LDFLAGS = $(COVERAGE_LDFLAGS)
+
EXTRA_DIST = $(conf_DATA)
BUILT_SOURCES =
@@ -420,20 +423,21 @@ libvirt_la_LIBADD = $(libvirt_la_BUILT_LIBADD)
libvirt_la_BUILT_LIBADD = libvirt_util.la
libvirt_util_la_SOURCES = \
$(UTIL_SOURCES)
-libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS)
+libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \
+ $(AM_CFLAGS)
libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS)
The code in libvirt that does ACS checking has a pretty
serious bug that was essentially rendering the check useless.
When trying to assign a device, we have to check that all
bridges upstream of this device support ACS. That means
that we have to find the parent of the current device,
check for ACS, then find the parent of that device, check
for ACS, etc.
However, the code to find the parent of the device had a
much too relaxed check. It would iterate through all PCI
devices on the system, looking for a device that had a range
of busses that included the current device's bus.
That's not correct, though. Depending on how we iterated
through the list of PCI devices, we could first find the
*topmost* bridge in the system; since it necessarily had
a range of busses including the current device's bus, we
would only ever check the topmost bridge, and not check
any of the intermediate bridges.
This patch is simple in that it looks for the PCI device
whose secondary device *exactly* equals the bus of the
device we are looking for. That means that one, and only one
bridge will be found, and it will be the correct device.
Note that this also caused a fairly serious bug in the
secondary bus reset code, where we could erroneously
reset the topmost bus instead of the inner bus.
This patch was tested by me on a 4-port NIC with a
bridge without ACS (where assignment failed), a 4-port
NIC with a bridge with ACS (where assignment succeeded),
and a 2-port NIC with no bridges (where assignment
succeeded).
Signed-off-by: Chris Lalancette
---
src/util/pci.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index 26d55b8..df30e04 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -513,7 +513,7 @@ static int
pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
{
uint16_t device_class;
-uint8_t header_type, secondary, subordinate;
+uint8_t header_type, secondary;
if (dev->domain != check->domain)
return 0;
@@ -529,12 +529,11 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data
ATTRIBUTE_UNUSED)
return 0;
secondary = pciRead8(check, PCI_SECONDARY_BUS);
-subordinate = pciRead8(check, PCI_SUBORDINATE_BUS);
VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name,
check->name);
/* No, it's superman! */
-return (dev->bus >= secondary && dev->bus <= subordinate);
+return (dev->bus == secondary);
}
static pciDevice *
--
1.7.1.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
If detecting the FLR flag of a pci device fails, then we
could run into the situation of trying to close a file
descriptor twice, once in pciInitDevice() and once in pciFreeDevice().
Fix that by removing the pciCloseConfig() in pciInitDevice() and
just letting pciFreeDevice() handle it.
Thanks to Chris Wright for pointing out this problem.
While we are at it, fix an error check. While it would actually
work as-is (since success returns 0), it's still more clear to
check for < 0 (as the rest of the code does).
Signed-off-by: Chris Lalancette
---
src/qemu/qemu_driver.c |2 +-
src/util/pci.c |8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b4c468a..9436e2a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8010,7 +8010,7 @@ static int qemudDomainAttachHostPciDevice(struct
qemud_driver *driver,
return -1;
}
-if (qemuPrepareHostdevPCIDevices(driver, &hostdev, 1))
+if (qemuPrepareHostdevPCIDevices(driver, &hostdev, 1) < 0)
return -1;
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
diff --git a/src/util/pci.c b/src/util/pci.c
index 14ef058..26d55b8 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -189,8 +189,10 @@ pciCloseConfig(pciDevice *dev)
if (!dev)
return;
-if (dev->fd >= 0)
+if (dev->fd >= 0) {
close(dev->fd);
+dev->fd = -1;
+}
}
static int
@@ -673,10 +675,8 @@ pciInitDevice(pciDevice *dev)
dev->pcie_cap_pos = pciFindCapabilityOffset(dev, PCI_CAP_ID_EXP);
dev->pci_pm_cap_pos = pciFindCapabilityOffset(dev, PCI_CAP_ID_PM);
flr = pciDetectFunctionLevelReset(dev);
-if (flr < 0) {
-pciCloseConfig(dev);
+if (flr < 0)
return flr;
-}
dev->has_flr= flr;
dev->has_pm_reset = pciDetectPowerManagementReset(dev);
dev->initted= 1;
--
1.7.1.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/28/2010 06:22 AM, Daniel P. Berrange wrote:
> On Mon, Jul 26, 2010 at 03:16:25PM -0400, Cole Robinson wrote:
>> Enable specifying a virtio console device with:
>>
>>
>>
>>
>>
>> Signed-off-by: Cole Robinson
>
> ACK, but one extra thing you need here I think. When I tested
> it I had to manually add a element. We should be
> auto-adding a for virtio serial if we get a
> virtio device, in same way we do for virtio
> devices.
>
Hmm, I can't reproduce: there is even an explicit unit test case for
this, so not sure what steps you took to trigger the issue.
I've pushed this series now, let me know if you can still reproduce.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/28/2010 01:24 PM, Daniel P. Berrange wrote:
On Wed, Jul 28, 2010 at 12:24:53PM -0500, Patrick Dignan wrote:
On 07/28/2010 05:08 AM, Daniel P. Berrange wrote:
On Tue, Jul 27, 2010 at 06:28:01PM -0500, Patrick Dignan wrote:
Hi Everyone,
I'm looking at implementing some functionality in libvirt that would
allow it to call functions in an unpublished iSCSI library. Some of the
functionality I wish to implement is not currently part of the libvirt
storage API. I wanted to suggest the following additions to the storage
API: grow volumes, show whether thin provisioning is enabled, enable
thin provisioning, disable thin provisioning, create snapshots, and
delete snapshots. I've added a patch at the end of the mail showing how
I think these functions should be implemented. Note that I have not
included details about the virStorageSnapshotDefPtr yet, that's the next
step.
Perhaps this should be in a separate mail for better threading, but it
seems a bit strange to me that the storage interface isn't pluggable in
the traditional sense. In order to add a backend to libvirt, one has to
make modifications all over the place, for example: virt-inst, the
Makefile.am, the configure.ac, storage_backend.h, and several other
places. It would make sense to me to make this pluggable such that
someone could just load in a library that implements the required
functions and some identifying information (eg type of storage,
description, etc). A list of supported backends could be stored in
empty files in a directory somewhere, or some similar hack. This way
someone could write a plugin for tgtd for example, or in my case the
library I'm working with. I think this would also help others with
writing plugins for more storage backends. How difficult do you think
this would be? I'm willing to do a reasonable amount of work to get
this implemented, but I want to know what the experts think!
We explicitly don't support external driver plugins in libvirt for a
couple of reasons
- We don't want to support use of closed source plugins
- We don't want to guarentee stability of any aspect of
libvirt's internal API
We would like to see support for the various vendor specific iSCSI
extensions to allow volume creation/deletion, but want that code to
be part of the libvirt codebase.
Understandable. I was thinking that there is currently no way to
specify a vendor of a storage backend. For example, an iSCSI vendor.
This makes it look like implementing vendor-specific extensions requires
creating a new backend, even though there's already an iSCSI backend.
It seems like a secondary field for vendor, and maybe even model, could
help this.
Yes, we'd want to add some kind of vendor and/or model tag to the
storage pool XML description, to indicate what variant of iSCSI
is to be used.
Makes sense to me! maybe something like:
In the pool source element would make sense.
/* File creation/cloning functions used for cloning between backends */
int virStorageBackendCreateRaw(virConnectPtr conn,
@@ -76,6 +83,12 @@
virStorageBackendCreateVol createVol;
virStorageBackendRefreshVol refreshVol;
virStorageBackendDeleteVol deleteVol;
+virStorageBackendGrowVol growVol;
I'd call this 'resizeVol' since there's no reason we can't also support
shrinking.
Do all backends support shrinking? I was under the impression shrinking
is not quite a universal feature, so it made sense to me to break this
out. If most backends support shrinking, it makes sense to use
resizeVol instead then.
Plain files, LVM, and disk partitions can all be shrunk, so I
think this is fine. If a particular storage type does not support
shrinking then it can raise VIR_ERR_NO_SUPPORT if the users tries
to shrink.
Alright sounds good, I'll change the patch to reflect that.
+virStorageBackendThinProvisionShow thinProvisionShow;
+virStorageBackendThinProvisionEnable thinProvisionEnable;
+virStorageBackendThinProvisionDisable thinProvisionDisable;
I'm not really liking this as a concept. The other storage drivers, and
indeed my iSCSI server, deal with thin provisioning on a per-volume basis
when creating the volume. The libvirt model is that if in the XML, then
value is zero then the user is requesting thin provisioning
of that volume. ie no storage allocated for it upfront. If
matches then the volume should be fully allocated upfront.
Sorry, that was a bit of a misnomer on my part, these functions are
intended to be used at the volume level. However, what if they want to
enable thin provisioning or disable it after the fact? Is that not
going to be supported? An example use case of enabling after the fact
is if they want to enable it, and then grow the volume to a larger
size. Disabling, of course, could be done at any time (eg for a speed
increase). Do you object to the thinProvisionShow function, in e
On Wed, Jul 28, 2010 at 12:24:53PM -0500, Patrick Dignan wrote:
> On 07/28/2010 05:08 AM, Daniel P. Berrange wrote:
> >On Tue, Jul 27, 2010 at 06:28:01PM -0500, Patrick Dignan wrote:
> >
> >>Hi Everyone,
> >>
> >>I'm looking at implementing some functionality in libvirt that would
> >>allow it to call functions in an unpublished iSCSI library. Some of the
> >>functionality I wish to implement is not currently part of the libvirt
> >>storage API. I wanted to suggest the following additions to the storage
> >>API: grow volumes, show whether thin provisioning is enabled, enable
> >>thin provisioning, disable thin provisioning, create snapshots, and
> >>delete snapshots. I've added a patch at the end of the mail showing how
> >>I think these functions should be implemented. Note that I have not
> >>included details about the virStorageSnapshotDefPtr yet, that's the next
> >>step.
> >>
> >>Perhaps this should be in a separate mail for better threading, but it
> >>seems a bit strange to me that the storage interface isn't pluggable in
> >>the traditional sense. In order to add a backend to libvirt, one has to
> >>make modifications all over the place, for example: virt-inst, the
> >>Makefile.am, the configure.ac, storage_backend.h, and several other
> >>places. It would make sense to me to make this pluggable such that
> >>someone could just load in a library that implements the required
> >>functions and some identifying information (eg type of storage,
> >>description, etc). A list of supported backends could be stored in
> >>empty files in a directory somewhere, or some similar hack. This way
> >>someone could write a plugin for tgtd for example, or in my case the
> >>library I'm working with. I think this would also help others with
> >>writing plugins for more storage backends. How difficult do you think
> >>this would be? I'm willing to do a reasonable amount of work to get
> >>this implemented, but I want to know what the experts think!
> >>
> >We explicitly don't support external driver plugins in libvirt for a
> >couple of reasons
> >
> > - We don't want to support use of closed source plugins
> > - We don't want to guarentee stability of any aspect of
> >libvirt's internal API
> >
> >We would like to see support for the various vendor specific iSCSI
> >extensions to allow volume creation/deletion, but want that code to
> >be part of the libvirt codebase.
> >
> >
> Understandable. I was thinking that there is currently no way to
> specify a vendor of a storage backend. For example, an iSCSI vendor.
> This makes it look like implementing vendor-specific extensions requires
> creating a new backend, even though there's already an iSCSI backend.
> It seems like a secondary field for vendor, and maybe even model, could
> help this.
Yes, we'd want to add some kind of vendor and/or model tag to the
storage pool XML description, to indicate what variant of iSCSI
is to be used.
> >> /* File creation/cloning functions used for cloning between backends */
> >> int virStorageBackendCreateRaw(virConnectPtr conn,
> >>@@ -76,6 +83,12 @@
> >> virStorageBackendCreateVol createVol;
> >> virStorageBackendRefreshVol refreshVol;
> >> virStorageBackendDeleteVol deleteVol;
> >>+virStorageBackendGrowVol growVol;
> >>
> >I'd call this 'resizeVol' since there's no reason we can't also support
> >shrinking.
> >
> Do all backends support shrinking? I was under the impression shrinking
> is not quite a universal feature, so it made sense to me to break this
> out. If most backends support shrinking, it makes sense to use
> resizeVol instead then.
Plain files, LVM, and disk partitions can all be shrunk, so I
think this is fine. If a particular storage type does not support
shrinking then it can raise VIR_ERR_NO_SUPPORT if the users tries
to shrink.
> >>+virStorageBackendThinProvisionShow thinProvisionShow;
> >>+virStorageBackendThinProvisionEnable thinProvisionEnable;
> >>+virStorageBackendThinProvisionDisable thinProvisionDisable;
> >>
> >I'm not really liking this as a concept. The other storage drivers, and
> >indeed my iSCSI server, deal with thin provisioning on a per-volume basis
> >when creating the volume. The libvirt model is that if in the XML, then
> > value is zero then the user is requesting thin provisioning
> >of that volume. ie no storage allocated for it upfront. If
> >matches then the volume should be fully allocated upfront.
> >
> >
> Sorry, that was a bit of a misnomer on my part, these functions are
> intended to be used at the volume level. However, what if they want to
> enable thin provisioning or disable it after the fact? Is that not
> going to be supported? An example use case of enabling after the fact
> is if they want to enable it, and then grow the volume to a larger
> size. Disabling, of course, could be done at any time (eg for a speed
> increase). Do you object to the thinProvisionShow funct
On Wed, Jul 28, 2010 at 03:22:50PM +0200, Matthias Bolte wrote:
>
> I think you're right, a vpx:// connection that covers a whole vCenter
> is probably out of libvirt's scope and gives much trouble with stuff
> like name uniqueness etc.
>
> I'm going to restrict a vpx:// connection to a single host for now:
>
> vpx://example-vcenter.com/datacenter1/cluster1/hostsystem1
>
> This solves all name uniqueness issues and it allows to resolve
> remaining no-host-for-vpx-connection-todos in the driver. This way we
> don't have a feature in 0.8.3 that might need to be restricted in
> later version.
>
> Once we have a single-host-vpx-connection I'd like to investigate if
> it's feasible to relax the restrictions on a vpx:// in a way that
> allows to manage a single cluster too. Internally the driver would
> always manage (Cluster-)ComputeResources then. The URI for such a
> connection would omit the host part:
>
> vpx://example-vcenter.com/datacenter1/cluster1
This all sounds like a good plan to me
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/28/2010 05:08 AM, Daniel P. Berrange wrote:
On Tue, Jul 27, 2010 at 06:28:01PM -0500, Patrick Dignan wrote:
Hi Everyone,
I'm looking at implementing some functionality in libvirt that would
allow it to call functions in an unpublished iSCSI library. Some of the
functionality I wish to implement is not currently part of the libvirt
storage API. I wanted to suggest the following additions to the storage
API: grow volumes, show whether thin provisioning is enabled, enable
thin provisioning, disable thin provisioning, create snapshots, and
delete snapshots. I've added a patch at the end of the mail showing how
I think these functions should be implemented. Note that I have not
included details about the virStorageSnapshotDefPtr yet, that's the next
step.
Perhaps this should be in a separate mail for better threading, but it
seems a bit strange to me that the storage interface isn't pluggable in
the traditional sense. In order to add a backend to libvirt, one has to
make modifications all over the place, for example: virt-inst, the
Makefile.am, the configure.ac, storage_backend.h, and several other
places. It would make sense to me to make this pluggable such that
someone could just load in a library that implements the required
functions and some identifying information (eg type of storage,
description, etc). A list of supported backends could be stored in
empty files in a directory somewhere, or some similar hack. This way
someone could write a plugin for tgtd for example, or in my case the
library I'm working with. I think this would also help others with
writing plugins for more storage backends. How difficult do you think
this would be? I'm willing to do a reasonable amount of work to get
this implemented, but I want to know what the experts think!
We explicitly don't support external driver plugins in libvirt for a
couple of reasons
- We don't want to support use of closed source plugins
- We don't want to guarentee stability of any aspect of
libvirt's internal API
We would like to see support for the various vendor specific iSCSI
extensions to allow volume creation/deletion, but want that code to
be part of the libvirt codebase.
Understandable. I was thinking that there is currently no way to
specify a vendor of a storage backend. For example, an iSCSI vendor.
This makes it look like implementing vendor-specific extensions requires
creating a new backend, even though there's already an iSCSI backend.
It seems like a secondary field for vendor, and maybe even model, could
help this.
/* File creation/cloning functions used for cloning between backends */
int virStorageBackendCreateRaw(virConnectPtr conn,
@@ -76,6 +83,12 @@
virStorageBackendCreateVol createVol;
virStorageBackendRefreshVol refreshVol;
virStorageBackendDeleteVol deleteVol;
+virStorageBackendGrowVol growVol;
I'd call this 'resizeVol' since there's no reason we can't also support
shrinking.
Do all backends support shrinking? I was under the impression shrinking
is not quite a universal feature, so it made sense to me to break this
out. If most backends support shrinking, it makes sense to use
resizeVol instead then.
+virStorageBackendThinProvisionShow thinProvisionShow;
+virStorageBackendThinProvisionEnable thinProvisionEnable;
+virStorageBackendThinProvisionDisable thinProvisionDisable;
I'm not really liking this as a concept. The other storage drivers, and
indeed my iSCSI server, deal with thin provisioning on a per-volume basis
when creating the volume. The libvirt model is that if in the XML, then
value is zero then the user is requesting thin provisioning
of that volume. ie no storage allocated for it upfront. If
matches then the volume should be fully allocated upfront.
Sorry, that was a bit of a misnomer on my part, these functions are
intended to be used at the volume level. However, what if they want to
enable thin provisioning or disable it after the fact? Is that not
going to be supported? An example use case of enabling after the fact
is if they want to enable it, and then grow the volume to a larger
size. Disabling, of course, could be done at any time (eg for a speed
increase). Do you object to the thinProvisionShow function, in either case?
+virStorageBackendCreateSnapshot createSnapshot;
+virStorageBackendDeleteSnapshot deleteSnapshot;
There's no need for snapshot APIs. This functionality is already supported
via the normal volume creation API, just specify the original volume to be
snapshotted in the XML as the backing store.
I wasn't aware of this functionality. It looks like it's implemented on
a per-hypervisor basis. It'd be really cool to get snapshotting
integrated into storage backends with snapshotting support, so that
snapshots would show up in both libvirt and the storage backend's UI,
but I can see how this would be nearly impossible.
Regards,
Dan
Hello,
I'd like to be able to add interfaces to OpenVZ guests like the following
vzctl command does : vzctl set 100 --netif_add eth0,00:11:22:33:44:55,tap0
Here, eth0 is the guest interface name, and tap0 the host interface name.
This is currently not supported by libvirt so I'd like to implement it.
In terms of domain XML I think it would be something like:
Currently the OpenVZ driver calls "vzctl set 100 --ipadd" when the interface
type is "ethernet" and the ip address attribute isn't empty.
Does it seem ok to use --netif_add for ethernet interfaces when there is no ip
defined ?
Regards,
Jean-Baptiste Rouault
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 28, 2010 at 03:17:29PM +0200, Gerhard Stenzel wrote:
> During function test of the 802.1Qbg implementation in lldpad we came
> across a small problem in the handling of the netlink message
> corresponding to PORT_PROFILE_RESPONSE_INPROGRESS. This should not
> result in returning the default rc=1.
>
>
> Signed-off-by: Gerhard Stenzel
>
> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index 635458d..a6d9a57 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -1025,6 +1025,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t
> vf,
> if (is8021Qbg) {
> /* no in-progress here; may be missing */
> *status = PORT_PROFILE_RESPONSE_INPROGRESS;
> +rc = 0;
> } else {
> msg = _("no IFLA_PORT_RESPONSE found in netlink message");
> goto err_exit;
ACK, looks sensible, I aslo took the opportunity to fix a small
indentation problem in that block (aligned on 5 instead of 4) and pushed
it,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
* src/network/bridge_driver.c
(networkAddMasqueradingIptablesRules): Fix spelling and grammar.
---
Pushing under the trivial change rule.
src/network/bridge_driver.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 80ed57a..d6c7ae1 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -643,12 +643,12 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
*
* We need to end up with 3 rules in the table in this order
*
- * 1. protocol=tcp with sport mapping restricton
- * 2. protocol=udp with sport mapping restricton
+ * 1. protocol=tcp with sport mapping restriction
+ * 2. protocol=udp with sport mapping restriction
* 3. generic any protocol
*
* The sport mappings are required, because default IPtables
- * MASQUERADE is maintain port number unchanged where possible.
+ * MASQUERADE maintain port numbers unchanged where possible.
*
* NFS can be configured to only "trust" port numbers < 1023.
*
--
1.7.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/28/2010 04:53 AM, Daniel P. Berrange wrote:
On Tue, Jul 27, 2010 at 12:20:55PM -0500, Anthony Liguori wrote:
On 07/27/2010 12:00 PM, Daniel P. Berrange wrote:
Yup. You'll need to decide up front if you want to probe for a feature
when it's introduced and have something added to capabilities.
This is simple though. A few weeks before 0.14 is released, go through
the change log, and anything that looks interesting, add a cap flag.
That doesn't work for features which already exist in QEMU which are
not yet supported in libvirt. eg consider QEMU 0.13 is released, and
then we want to add a new feature to libvirt a month later.
Right. So sit down and look at the 0.13 changelog and if there's any
features that you think you might want to support at some point in time,
add a capability.
Not really practical - pretty much anything is a candidate because
we want to support as many of QEMU features as possible.
It offers significantly less information that the existing -help
data, so I don't think it is workable as a replacement. We'd get
into the bad situation where we could support a feature in 0.12
but not in 0.13, unless we went back to using -help output again.
If we're going for a short term hack, then how about a combination
of
http://www.mail-archive.com/qemu-de...@nongnu.org/msg34944.html
http://www.mail-archive.com/qemu-de...@nongnu.org/msg34960.html
Would have failed in exactly the same way that the current -help parsing
fails. The description of an argument in the help text is not a
capabilities string.
This particular case of cache modes might have failed, but in the
broader picture it would have been more reliable. The vasty
majority of the time, we only check whether a particular named
argument exists. This patch would make it very reliable todo so
because you could simply extract a single named field from the
JSON doc. The cases where we looked at the parameter options
would have been improved a little, but still be potentially fragile.
Checking for version number would also be improved. So overall
while obviously not perfect, it would be significantly better than
the solution today and using an approach that isn't a complete
throwaway solution.
I'd be willing to spit out the option names minus the help
descriptions. The option names are part of a supported interface so
there's no harm in exposing an interface for that.
But I think libvirt really needs option names + indication when we've
added parameters to an option, right?
Regards,
Anthony Liguori
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2010/7/26 Daniel Veillard :
> On Sat, Jul 24, 2010 at 10:36:50PM +0200, Matthias Bolte wrote:
>> 2010/7/23 Eric Blake :
> [...]
>> From: Eric Blake
>> Date: Fri, 23 Jul 2010 14:28:31 -0600
>> Subject: [PATCH] maint: turn on gcc logical-op checking
>>
>> This would have detected the bug in commit 38ad33931 (Aug 09), which
>> we missed until commit f828ca35 (Jul 10); over 11 months later.
>>
>> However, on Fedora 13, it also triggers LOTS of warnings from
>> the libcurl-devel header for two files:
>>
>> esx/esx_vi.c: In function 'esxVI_CURL_Perform':
>> esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always
>> evaluate as true [-Wlogical-op]
>> esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always
>> evaluate as true [-Wlogical-op]
>> esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always
>> evaluate as true [-Wlogical-op]
>> ...
>> xenapi/xenapi_driver.c: In function 'call_func':
>> xenapi/xenapi_driver.c:1872: error: logical '&&' with non-zero constant will
>> always evaluate as true [-Wlogical-op]
>> xenapi/xenapi_driver.c:1872: error: logical '&&' with non-zero constant will
>> always evaluate as true [-Wlogical-op]
>> xenapi/xenapi_driver.c:1872: error: logical '&&' with non-zero constant will
>> always evaluate as true [-Wlogical-op]
>> ...
>
> ACK to Matthias' patch, is there is a predefined way to avoid those
> defines, let's use it !
>
> Daniel
>
Thanks, pushed.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/28/2010 07:05 AM, Daniel Veillard wrote:
> On Wed, Jul 28, 2010 at 11:04:34AM +0200, Jiri Denemark wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=617300 documents
>>> several shortcoming in our libvirt-guests init script, in
>>> relation to Fedora init script standards. For easier review,
>>> I've split it into three simpler patches.
>>>
>>> Eric Blake (3):
>>> libvirt-guests: detect invalid arguments
>>> libvirt-guests: enhance status
>>> libvirt-guests: add reload, condrestart
>>
>> ACK to all of them.
>
> Looks fine to me too, ACK,
Thanks; pushed.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2010/7/28 Daniel P. Berrange :
> On Tue, Jul 27, 2010 at 08:39:50PM +0200, Matthias Bolte wrote:
>> 2010/7/26 Daniel P. Berrange :
>> > On Mon, Jul 19, 2010 at 01:26:10AM +0200, Matthias Bolte wrote:
>> >> Add a pointer to the primary context of a connection and use it in all
>> >> driver functions that don't dependent on the context type. This includes
>> >> almost all functions that deal with a virDomianPtr. Therefore, using
>> >> a vpx:// connection allows you to perform all the usual domain related
>> >> actions like start, destroy, suspend, resume, dumpxml etc.
>> >>
>> >> Some functions that require an explicitly specified ESX server don't work
>> >> yet. This includes the host UUID, the hostname, the general node info, the
>> >> max vCPU count and the free memory. Also not working yet are migration and
>> >> defining new domains.
>> >
>> >
>> > If you're connecting to vpx://example-vcenter.com how does the driver
>> > know which host you're asking for data on ? IIUC a vcenter reports on all
>> > hosts in a data center. Does new mode still guarentee that every domain
>> > has a unique name & ID, as well as UUID ?
>> >
>>
>> UUID is unique by definition.
>>
>> The driver parses the ID from the internal object name. As far as I
>> understand the object model this ID is unique. But the ID might be
>> different if you look at the same guest through a esx:// or vpx://
>> connection, but it's unique per connection.
>>
>> The name is a bit more tricky. For a single ESX server it's unique. I
>> thought for a vCenter it's unique per datacenter, because the vCenter
>> won't let you define a second guest with an existing name. I just
>> played a bit with it and it seems there are ways to define two domains
>> with the same name in a single cluster. This is definitely a problem
>> and I'm not sure how to fix that, other than using hacks like adding
>> the ID to the name.
>>
>> In order to have a guaranteed unique domain name for a vpx://
>> connection the final name will probably have to be build like this
>>
>> //-
>>
>> Quite ugly and unstable, because it'll change across a migration :(
>>
>> The same name uniqueness problem exists for datastores, because their
>> names are unique per datacenter only.
>
> I guess what I'm getting at this is that libvirt is really providing a
> per-host level view of virt. When using vCenter I was expecting that
> it was still scoped to the host. ie just using vCenter as a means to
> control an individual host, not using vCenter for controlling all hosts
> at once. If we go for the latter we're turning libvirt into something
> more like DeltaCloud which I'm not convinced we want todo
>
> Regards,
> Daniel
>
I think you're right, a vpx:// connection that covers a whole vCenter
is probably out of libvirt's scope and gives much trouble with stuff
like name uniqueness etc.
I'm going to restrict a vpx:// connection to a single host for now:
vpx://example-vcenter.com/datacenter1/cluster1/hostsystem1
This solves all name uniqueness issues and it allows to resolve
remaining no-host-for-vpx-connection-todos in the driver. This way we
don't have a feature in 0.8.3 that might need to be restricted in
later version.
Once we have a single-host-vpx-connection I'd like to investigate if
it's feasible to relax the restrictions on a vpx:// in a way that
allows to manage a single cluster too. Internally the driver would
always manage (Cluster-)ComputeResources then. The URI for such a
connection would omit the host part:
vpx://example-vcenter.com/datacenter1/cluster1
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
During function test of the 802.1Qbg implementation in lldpad we came
across a small problem in the handling of the netlink message
corresponding to PORT_PROFILE_RESPONSE_INPROGRESS. This should not
result in returning the default rc=1.
Signed-off-by: Gerhard Stenzel
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 635458d..a6d9a57 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -1025,6 +1025,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t
vf,
if (is8021Qbg) {
/* no in-progress here; may be missing */
*status = PORT_PROFILE_RESPONSE_INPROGRESS;
+rc = 0;
} else {
msg = _("no IFLA_PORT_RESPONSE found in netlink message");
goto err_exit;
--
Best regards,
Gerhard Stenzel,
---
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jul 27, 2010 at 10:55:03AM -0500, Anthony Liguori wrote:
> Today libvirt parses -help output to attempt to enumerate capabilities. This
> is very broken and has led to multiple failures. Since libvirt is an
> important
> management interface to QEMU, we need to do a better job giving them the
> ability
> to detect what a QEMU executable supports. Right now, we keep fixing up help
> output to appease it's parsing code but this is undesirable.
>
> The Right Solution is to introduce a robust capabilities advertisement that
> enumerates every feature we have. As with most Right Solutions, we don't have
> mergable code today and it's unclear that we'll get there by the next release.
>
> This patch introduces an incremental solution of just spitting out the handful
> of capabilities libvirt is probing for today. This interface will need to
> remain forever but can stop being updated once we have a Right Solution.
Unfortunately, that doesn't sounds like a solution to me either,
we need the capabilities support in the long term, and we will need
to keep the help output parsing code for legacy support. That mean
adding support for a third way, just for one or 2 release of QEmu.
So even if this was sufficient for libvirt as a temporary measure
I'm afraid the burden and extra code maintainance/testing etc. is not
worth it. We can live with help output parsing until the correct
solution is finalized, and it's IMHO better than supporting forever
a temporary half cooked solution both in libvirt and in QEmu.
thanks,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 26, 2010 at 03:16:25PM -0400, Cole Robinson wrote:
> Enable specifying a virtio console device with:
>
>
>
>
>
> Signed-off-by: Cole Robinson
ACK, but one extra thing you need here I think. When I tested
it I had to manually add a element. We should be
auto-adding a for virtio serial if we get a
virtio device, in same way we do for virtio
devices.
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 26, 2010 at 03:16:24PM -0400, Cole Robinson wrote:
> All devices now export a type attribute. QEMU defaults
> to 'serial', UML defaults to 'uml, xen can be either 'serial' or 'xen'
> depending on fullvirt. Understandably there is lots of test fallout.
>
> This will be used to differentiate between a serial vs. virtio console for
> QEMU.
>
ACK
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 26, 2010 at 03:16:22PM -0400, Cole Robinson wrote:
> There is actually a difference between the character device type (serial,
> parallel, channel, ...) and the target type (virtio, guestfwd). Currently
> they are awkwardly conflated.
>
> Start to pull them apart by renaming targetType -> deviceType. This is
> an entirely mechanical change.
>
> Signed-off-by: Cole Robinson
ACK
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jul 27, 2010 at 06:28:01PM -0500, Patrick Dignan wrote:
> Hi Everyone,
>
> I'm looking at implementing some functionality in libvirt that would
> allow it to call functions in an unpublished iSCSI library. Some of the
> functionality I wish to implement is not currently part of the libvirt
> storage API. I wanted to suggest the following additions to the storage
> API: grow volumes, show whether thin provisioning is enabled, enable
> thin provisioning, disable thin provisioning, create snapshots, and
> delete snapshots. I've added a patch at the end of the mail showing how
> I think these functions should be implemented. Note that I have not
> included details about the virStorageSnapshotDefPtr yet, that's the next
> step.
>
> Perhaps this should be in a separate mail for better threading, but it
> seems a bit strange to me that the storage interface isn't pluggable in
> the traditional sense. In order to add a backend to libvirt, one has to
> make modifications all over the place, for example: virt-inst, the
> Makefile.am, the configure.ac, storage_backend.h, and several other
> places. It would make sense to me to make this pluggable such that
> someone could just load in a library that implements the required
> functions and some identifying information (eg type of storage,
> description, etc). A list of supported backends could be stored in
> empty files in a directory somewhere, or some similar hack. This way
> someone could write a plugin for tgtd for example, or in my case the
> library I'm working with. I think this would also help others with
> writing plugins for more storage backends. How difficult do you think
> this would be? I'm willing to do a reasonable amount of work to get
> this implemented, but I want to know what the experts think!
We explicitly don't support external driver plugins in libvirt for a
couple of reasons
- We don't want to support use of closed source plugins
- We don't want to guarentee stability of any aspect of
libvirt's internal API
We would like to see support for the various vendor specific iSCSI
extensions to allow volume creation/deletion, but want that code to
be part of the libvirt codebase.
> /* File creation/cloning functions used for cloning between backends */
> int virStorageBackendCreateRaw(virConnectPtr conn,
> @@ -76,6 +83,12 @@
> virStorageBackendCreateVol createVol;
> virStorageBackendRefreshVol refreshVol;
> virStorageBackendDeleteVol deleteVol;
> +virStorageBackendGrowVol growVol;
I'd call this 'resizeVol' since there's no reason we can't also support
shrinking.
> +virStorageBackendThinProvisionShow thinProvisionShow;
> +virStorageBackendThinProvisionEnable thinProvisionEnable;
> +virStorageBackendThinProvisionDisable thinProvisionDisable;
I'm not really liking this as a concept. The other storage drivers, and
indeed my iSCSI server, deal with thin provisioning on a per-volume basis
when creating the volume. The libvirt model is that if in the XML, then
value is zero then the user is requesting thin provisioning
of that volume. ie no storage allocated for it upfront. If
matches then the volume should be fully allocated upfront.
> +virStorageBackendCreateSnapshot createSnapshot;
> +virStorageBackendDeleteSnapshot deleteSnapshot;
There's no need for snapshot APIs. This functionality is already supported
via the normal volume creation API, just specify the original volume to be
snapshotted in the XML as the backing store.
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jul 27, 2010 at 12:20:55PM -0500, Anthony Liguori wrote:
> On 07/27/2010 12:00 PM, Daniel P. Berrange wrote:
> >>Yup. You'll need to decide up front if you want to probe for a feature
> >>when it's introduced and have something added to capabilities.
> >>
> >>This is simple though. A few weeks before 0.14 is released, go through
> >>the change log, and anything that looks interesting, add a cap flag.
> >>
> >That doesn't work for features which already exist in QEMU which are
> >not yet supported in libvirt. eg consider QEMU 0.13 is released, and
> >then we want to add a new feature to libvirt a month later.
>
> Right. So sit down and look at the 0.13 changelog and if there's any
> features that you think you might want to support at some point in time,
> add a capability.
Not really practical - pretty much anything is a candidate because
we want to support as many of QEMU features as possible.
> >It offers significantly less information that the existing -help
> >data, so I don't think it is workable as a replacement. We'd get
> >into the bad situation where we could support a feature in 0.12
> >but not in 0.13, unless we went back to using -help output again.
> >
> >If we're going for a short term hack, then how about a combination
> >of
> >
> >http://www.mail-archive.com/qemu-de...@nongnu.org/msg34944.html
> >http://www.mail-archive.com/qemu-de...@nongnu.org/msg34960.html
> >
>
> Would have failed in exactly the same way that the current -help parsing
> fails. The description of an argument in the help text is not a
> capabilities string.
This particular case of cache modes might have failed, but in the
broader picture it would have been more reliable. The vasty
majority of the time, we only check whether a particular named
argument exists. This patch would make it very reliable todo so
because you could simply extract a single named field from the
JSON doc. The cases where we looked at the parameter options
would have been improved a little, but still be potentially fragile.
Checking for version number would also be improved. So overall
while obviously not perfect, it would be significantly better than
the solution today and using an approach that isn't a complete
throwaway solution.
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jul 27, 2010 at 08:39:50PM +0200, Matthias Bolte wrote:
> 2010/7/26 Daniel P. Berrange :
> > On Mon, Jul 19, 2010 at 01:26:10AM +0200, Matthias Bolte wrote:
> >> Add a pointer to the primary context of a connection and use it in all
> >> driver functions that don't dependent on the context type. This includes
> >> almost all functions that deal with a virDomianPtr. Therefore, using
> >> a vpx:// connection allows you to perform all the usual domain related
> >> actions like start, destroy, suspend, resume, dumpxml etc.
> >>
> >> Some functions that require an explicitly specified ESX server don't work
> >> yet. This includes the host UUID, the hostname, the general node info, the
> >> max vCPU count and the free memory. Also not working yet are migration and
> >> defining new domains.
> >
> >
> > If you're connecting to vpx://example-vcenter.com how does the driver
> > know which host you're asking for data on ? IIUC a vcenter reports on all
> > hosts in a data center. Does new mode still guarentee that every domain
> > has a unique name & ID, as well as UUID ?
> >
>
> UUID is unique by definition.
>
> The driver parses the ID from the internal object name. As far as I
> understand the object model this ID is unique. But the ID might be
> different if you look at the same guest through a esx:// or vpx://
> connection, but it's unique per connection.
>
> The name is a bit more tricky. For a single ESX server it's unique. I
> thought for a vCenter it's unique per datacenter, because the vCenter
> won't let you define a second guest with an existing name. I just
> played a bit with it and it seems there are ways to define two domains
> with the same name in a single cluster. This is definitely a problem
> and I'm not sure how to fix that, other than using hacks like adding
> the ID to the name.
>
> In order to have a guaranteed unique domain name for a vpx://
> connection the final name will probably have to be build like this
>
> //-
>
> Quite ugly and unstable, because it'll change across a migration :(
>
> The same name uniqueness problem exists for datastores, because their
> names are unique per datacenter only.
I guess what I'm getting at this is that libvirt is really providing a
per-host level view of virt. When using vCenter I was expecting that
it was still scoped to the host. ie just using vCenter as a means to
control an individual host, not using vCenter for controlling all hosts
at once. If we go for the latter we're turning libvirt into something
more like DeltaCloud which I'm not convinced we want todo
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> https://bugzilla.redhat.com/show_bug.cgi?id=617300 documents
> several shortcoming in our libvirt-guests init script, in
> relation to Fedora init script standards. For easier review,
> I've split it into three simpler patches.
>
> Eric Blake (3):
> libvirt-guests: detect invalid arguments
> libvirt-guests: enhance status
> libvirt-guests: add reload, condrestart
ACK to all of them.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On (Tue) Jul 27 2010 [10:55:03], Anthony Liguori wrote:
> +case QEMU_OPTION_libvirt_caps:
> +printf("version: " QEMU_VERSION "\n"
> + "package: " QEMU_PKGVERSION "\n"
> + "caps: name,enable-kvm,no-reboot,uuid,xen-domid,drive"
> + ",cache-v2,format,vga,serial,mem-path,chardev,balloon"
> + ",device,rtc,netdev,sdl,topology\n");
> +exit(0);
> +break;
This can't work; people have to remember not only to update
documentation but also this case here to ensure libvirt works fine.
There are some other options that might work:
- making such a list by taking the savevm section name and version
number for each device
- The parameters supported by devices registered with qdev (and their
defaults)
- etc.
But basically this means libvirt will have to do more work now and also
add support for capabilities later. We can instead just keep 0.13 as it
is and move to capabilities for 0.14.
Amit
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
