Commit Graph

13750 Commits

Author SHA1 Message Date
Pavel Hrdina
a16e5f0a91 qemu: check defaultMode for spice graphics independently
Instead of checking defaultMode for every channel that has no mode
configured, test it only once outside of channel loop.  This fixes a bug
that in case all possible channels are fore example set to insecure, but
defaultMode is set to secure, we wouldn't auto-generate TLS port.  This
results in failure while starting a guest.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143832

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2015-03-03 11:42:33 +01:00
Pavel Hrdina
e4983952b4 qemu: remove duplicated code for allocating spice ports
We have two different places that needs to be updated while touching
code for allocation spice ports.  Add a bool option to
'qemuProcessSPICEAllocatePorts' function to switch between true and fake
allocation so we can use this function also in qemu_driver to generate
native domain definition.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2015-03-03 11:41:46 +01:00
Martin Kletzander
2fd5880b3b conf: De-duplicate scheduling policy enums
Since adding the support for scheduler policy settings in commit
8680ea97, there are two enums with the same information.  That was
caused by rewriting the patch since first draft.

Find out thanks to clang, but there was no impact whatsoever.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-03-03 09:26:59 +01:00
Martin Kletzander
b3ea0a8fb8 qemu: Don't crash in qemuDomainOpenChannel()
The problem here was that when opening a channel, we were checking
whether the channel given is alias (can't be NULL for running domain) or
it's name, which can be NULL (for example with spicevmc).  In case of
such domain qemuDomainOpenChannel() made the daemon crash.
STREQ_NULLABLE() is safe to use since the code in question is wrapped in
"if (name)" and is more readable, so use that instead of checking for
non-NULL "vm->def->channels[i]->target.name".

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-03-03 09:26:59 +01:00
John Ferlan
30f69ae86b iscsi: Adjust error message for findStorageSources backend
The virStorageBackendISCSIFindPoolSources API only needs the 'host' name
in order to discover iSCSI pools, it returns the various device paths.
On input, it's also possible to further restrict a search by providing the
port attribute for the host element and the (undocumented) initiator element.

For example:

$  virsh find-storage-pool-sources-as iscsi
error: Failed to find any iscsi pool sources
error: invalid argument: hostname and device path must be specified for iscsi sources

$ virsh find-storage-pool-sources-as iscsi 192.168.122.1
<sources>
  <source>
    <host name='192.168.122.1' port='3260'/>
    <device path='iqn.2013-12.com.example:iscsi-chap-lclpool'/>
  </source>
</sources>
2015-03-02 22:57:27 -05:00
John Ferlan
832a9256b2 disk: Provide a default storage source format type.
https://bugzilla.redhat.com/show_bug.cgi?id=1181062

According to the formatstorage.html description for <source> element
and "format" attribute: "All drivers are required to have a default
value for this, so it is optional."

As it turns out the disk backend did not choose a default value, so I
added a default of "msdos" if the source type is "unknown" as well as
updating the storage.html backend disk volume driver documentation to
indicate the default format is dos.
2015-03-02 22:42:25 -05:00
John Ferlan
e0e290552b disk: Disallow duplicated target 'dev' values
https://bugzilla.redhat.com/show_bug.cgi?id=1142631

This patch resolves a situation where the same "<target dev='$name'...>"
can be used for multiple disks in the domain.

While the $name is "mostly" advisory regarding the expected order that
the disk is added to the domain and not guaranteed to map to the device
name in the guest OS, it still should be unique enough such that other
domblk* type operations can be performed.

Without the patch, the domblklist will list the same Target twice:

$ virsh domblklist $dom
Target     Source
------------------------------------------------
sda        /var/lib/libvirt/images/file.qcow2
sda        /var/lib/libvirt/images/file.img

Additionally, getting domblkstat, domblkerror, domblkinfo, and other block*
type calls will not be able to reference the second target.

Fortunately, hotplug disallows adding a "third" sda value:

$ qemu-img create -f raw /var/lib/libvirt/images/file2.img 10M
$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sda
error: Failed to attach disk
error: operation failed: target sda already exists

$

BUT, it since 'sdb' doesn't exist one would get the following on the same
hotplug attempt, but changing to use 'sdb' instead of 'sda'

$ virsh attach-disk $dom /var/lib/libvirt/images/file2.img sdb
error: Failed to attach disk
error: internal error: unable to execute QEMU command 'device_add': Duplicate ID 'scsi0-0-1' for device

$

Since we cannot fix this issue at parsing time, the best that can be done so
as to not "lose" a domain is to make the check prior to starting the guest
with the results as follows:

$ virsh start $dom
error: Failed to start domain $dom
error: XML error: target 'sda' duplicated for disk sources '/var/lib/libvirt/images/file.qcow2' and '/var/lib/libvirt/images/file.img'

$

Running 'make check' found a few more instances in the tests where this
duplicated target dev value was being used. These also exhibited some
duplicated 'id=' values (negating the uniqueness argument of aliases) in
the corresponding .args file and of course the *xmlout version of a few
input XML files.
2015-03-02 22:38:36 -05:00
Peter Krempa
8968c4946c conf: numa: Add helper to count total memory size configured in NUMA
The total NUMA memory consists of the sum of individual NUMA node memory
amounts.
2015-03-02 16:42:51 +01:00
Peter Krempa
389634e667 qemu: Forbid setting maximum memory size with the API with NUMA enabled
NUMA enabled guest configuration explicitly specifies memory sizes for
individual nodes. Allowing the virDomainSetMemoryFlags API (and friends)
to change the total doesn't make sense as the individual node configs
are not updated in that case.

Forbid use of the API in case NUMA is specified.
2015-03-02 16:41:32 +01:00
Peter Krempa
812a9e5e66 phyp: Fix error messages mentioning memory
The messages for currentMemory and memory were swapped.
2015-03-02 14:39:58 +01:00
Peter Krempa
1fcb9351d7 storage: sheepdog: Avoid skipping variable initialization
Commit 155ca616eb added a error message
that skips initialization of the 'cmd' variable. Fortunately it was not
released.
2015-03-02 10:09:49 +01:00
Ján Tomko
723522328f Check if domain is running in qemuDomainAgentIsAvailable
If the domain is not running, the agent will not respond.
Do not even try.

https://bugzilla.redhat.com/show_bug.cgi?id=872424
2015-03-02 08:07:56 +01:00
Ján Tomko
fbb94044ea Pass virDomainObjPtr to qemuDomainAgentAvailable
Not just the DomainObj's private data.
2015-03-02 08:07:56 +01:00
Ján Tomko
c8b80b49f6 Check for qemu guest agent availability after getting the job
This way checks requiring the job can be done in qemuDomainAgentAvailable.
2015-03-02 08:07:56 +01:00
Ján Tomko
155ca616eb Allow creating volumes with a backing store but no capacity
The tool creating the image can get the capacity from the backing
storage. Just refresh the volume afterwards.

https://bugzilla.redhat.com/show_bug.cgi?id=958510
2015-03-02 08:07:11 +01:00
Ján Tomko
813e4b7f83 Allow omitting volume capacity when backing store is specified
Add VIR_VOL_XML_PARSE_OPT_CAPACITY flag to virStorageVolDefParseXML.
With this flag, no error is reported when the capacity is missing
if there is a backing store.
2015-03-02 08:07:11 +01:00
Ján Tomko
d3452a3f73 Revert "Restore skipping of setting capacity"
This reverts commit f1856eb622.

Now that we can update capacity from image metadata,
we don't need to skip the update.
2015-03-02 08:07:11 +01:00
Ján Tomko
a760ba3a7f Probe for capacity in virStorageBackendUpdateVolTargetInfo
Instead of just looking at the output of fstat, call
virStorageFileGetMetadata to get the full capacity from
image headers.

Note that the capacity is probed unconditionally. The updateCapacity
bool parameter is ignored and will be removed in the following commit.
2015-03-02 08:07:11 +01:00
Ján Tomko
e3f1d2a820 Allow cloning volumes with no capacity specified
In virStorageVolCreateXML, add VIR_VOL_XML_PARSE_NO_CAPACITY
to the call parsing the XML of the new volume to make the capacity
optional.

If the capacity is omitted, use the capacity of the old volume.
We already do that for values that are less than the original
volume capacity.
2015-03-02 08:07:11 +01:00
Ján Tomko
2bd47d9c64 Allow parsing volumes without specifying the capacity
Add VIR_VOL_XML_PARSE_NO_CAPACITY flag to the volume XML
parser. When set, it allows the capacity element to be omitted.
2015-03-02 08:07:11 +01:00
Ján Tomko
21f58a5854 Parse backingStore before capacity in volume XML
So we can allow omitting the capacity element if backing store is
present.
2015-03-02 08:07:11 +01:00
Ján Tomko
cbd788eba6 Add flags argument to virStorageVolDefParse*
Allow the callers to pass down libvirt-internal flags.
2015-03-02 08:07:11 +01:00
Ján Tomko
000b21da83 Trivially implement VIR_CONNECT_BASELINE_CPU_MIGRATABLE for non-x86 cpus
Assume no features block migration.
2015-03-02 07:59:12 +01:00
Ján Tomko
496156807b Implement VIR_CONNECT_BASELINE_CPU_MIGRATABLE in the x86 cpu driver
Filter out non-migratable features if
VIR_CONNECT_BASELINE_CPU_MIGRATABLE was specified.
2015-03-02 07:59:12 +01:00
Ján Tomko
02c8b66d6a Add VIR_CONNECT_BASELINE_CPU_MIGRATABLE flag
This flag for virConnectBaselineCPU will allow filtering out
CPU features that block migration from the result.

https://bugzilla.redhat.com/show_bug.cgi?id=1171484
2015-03-02 07:59:12 +01:00
Ján Tomko
995ca6cbf3 Use virBufferTrim when generating boot options
Instead of tracking the number of added parameters,
add a comma at the end of each one unconditionally
and trim the trailing one at the end.
2015-03-02 07:39:09 +01:00
Ján Tomko
354425dcd2 Make -boot arg generation more readable
If we combine the boot order on the command line with other
boot options, we prepend order= in front of it.

Instead of checking if the number of added arguments is between
0 and 2, separate the strings for boot order and options
and prepend boot order only if both strings are not empty.
2015-03-02 07:39:09 +01:00
Ján Tomko
92572c3d71 Remove code handling the QEMU_CAPS_DOMID capability
This option is xenner-only (since commit b81a7ece),
and we dropped support for xenner in commit de9be0a.
2015-03-02 07:39:09 +01:00
Ján Tomko
9aa316612a Remove bootloader option from QEMU
It was only supported by xenner (since commit 763a59d8),
for which we removed support in commit de9be0a.

Remove the code generating this command line option,
refuse to parse it and delete the outdated tests.

https://bugzilla.redhat.com/show_bug.cgi?id=1176050
2015-03-02 07:39:09 +01:00
Ján Tomko
c75f42f331 Really fix XML formatting flags in SaveImageUpdateDef
Commit cf2d4c6 used a logical or instead of bitwise or,
effectively passing 1, that is VIR_DOMAIN_XML_INACTIVE.

This was caught by a warning when building with clang.

https://bugzilla.redhat.com/show_bug.cgi?id=1183869
2015-02-27 12:01:31 +01:00
Ján Tomko
8c45e8bec8 Ignore listen attribute of <graphics> for type network listens
Commit 6992994 started filling the listen attribute
of the parent <graphics> elements from type='network' listens.

When this XML is passed to UpdateDevice, parsing fails:
XML error: graphics listen attribute 10.20.30.40 must match
address attribute of first listen element (found none)

Ignore the address in the parent <graphics> attribute
when no type='address' listens are found,
the same we ignore the address for the <listen> subelements
when parsing inactive XML.
2015-02-27 08:27:21 +01:00
Peter Krempa
ef2e6f4089 util: storage: Fix error type in virStorageSourceParseBackingURI
The gluster volume name extraction code was copied from the XML parser
without changing the VIR_ERR_XML_ERROR error code. Use
VIR_ERR_CONFIG_UNSUPPORTED instead.
2015-02-26 11:50:38 +01:00
Peter Krempa
fc56ecd735 util: storagefile: Don't crash on gluster URIs without path
Similar to commit fdb80ed4f6 libvirtd
would crash if a gluster URI without path would be used in the backing
chain of a volume. The crash happens in the gluster specific part of the
parser that extracts the gluster volume name from the path.

Fix the crash by checking that the PATH is NULL.

This patch does not contain a test case as it's not possible to test it
with the current infrastructure as the test suite would attempt to
contact the gluster server in the URI. I'm working on the test suite
addition but that will be post-release material.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1196528
2015-02-26 11:50:38 +01:00
Ján Tomko
b15b21f3a5 conf: error out on missing dhcp host attributes
In virNetworkDHCPHostDefParseXML an error is reported
when partialOkay == true, and none of ip, mac, name
were supplied.

Add the missing goto and error out in this case.
2015-02-26 09:03:09 +01:00
Luyao Huang
719cd2182b conf: error out on invalid host id
https://bugzilla.redhat.com/show_bug.cgi?id=1196503

We already check whether the host id is valid or not, add a jump
to forbid invalid host id.

Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2015-02-26 08:52:43 +01:00
Laine Stump
4bbe1029f2 qemu: fix ifindex array reported to systemd
Commit f7afeddc added code to report to systemd an array of interface
indexes for all tap devices used by a guest. Unfortunately it not only
didn't add code to report the ifindexes for macvtap interfaces
(interface type='direct') or the tap devices used by type='ethernet',
it ended up sending "-1" as the ifindex for each macvtap or hostdev
interface. This resulted in a failure to start any domain that had a
macvtap or hostdev interface (or actually any type other than
"network" or "bridge").

This patch does the following with the nicindexes array:

1) Modify qemuBuildInterfaceCommandLine() to only fill in the
nicindexes array if given a non-NULL pointer to an array (and modifies
the test jig calls to the function to send NULL). This is because
there are tests in the test suite that have type='ethernet' and still
have an ifname specified, but that device of course doesn't actually
exist on the test system, so attempts to call virNetDevGetIndex() will
fail.

2) Even then, only add an entry to the nicindexes array for
appropriate types, and to do so for all appropriate types ("network",
"bridge", and "direct"), but only if the ifname is known (since that
is required to call virNetDevGetIndex().
2015-02-25 13:11:14 -05:00
Laine Stump
153b06c6d1 util: check for null ifname inside virNetDevBandwidthSet()
Previously this function relied on having ATTRIBUTE_NONNULL(1) in its
prototype rather than explicitly checking for a null
ifname. Unfortunately, ATTRIBUTE_NONNULL is just a hint to the
optimizer and code analyzers like Coverity, it doesn't actually check
anything at execution time, so the result was possible warnings from
Coverity, along with the possibility of null dereferences when ifname
wasn't available.

This patch removes the ATTRIBUTE_NONNULL from the prototype, and
checks ifname inside the function, logging an error if it's NULL (once
we've determined that the user really is trying to set a bandwidth).
2015-02-25 13:10:34 -05:00
Laine Stump
118b240808 network: only clear bandwidth if it has been set
libvirt was unconditionally calling virNetDevBandwidthClear() for
every interface (and network bridge) of a type that supported
bandwidth, whether it actually had anything set or not. This doesn't
hurt anything (unless ifname == NULL!), but is wasteful.

This patch makes sure that all calls to virNetDevBandwidthClear() are
qualified by checking that the interface really had some bandwidth
setup done, and checks for a null ifname inside
virNetDevBandwidthClear(), silently returning success if it is null
(as well as removing the ATTRIBUTE_NONNULL from that function's
prototype, since we can't guarantee that it is never null,
e.g. sometimes a type='ethernet' interface has no ifname as it is
provided on the fly by qemu).
2015-02-25 13:09:34 -05:00
Yuri Chornoivan
8a833d1eb0 Fix typos in messages
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2015-02-25 14:12:51 +01:00
Ján Tomko
6784acc7b0 Fix error messages in virStorageFileGetMetadataFromFD
Do not use relPath, it has not been filled by virStorageFileMetadataNew.
2015-02-25 12:14:30 +01:00
Ján Tomko
c6807b507a Clarify behavior or virDomainDetachDevice
Document that a complete device definition should be used
and a partial match can lead to the device being detached.

https://bugzilla.redhat.com/show_bug.cgi?id=872028
2015-02-25 10:06:41 +01:00
Ján Tomko
52a166f493 Assign default SCSI controller model before checking attribute validity
If the qemu binary on x86 does not support lsi SCSI controller,
but it supports virtio-scsi, we reject the virtio-specific attributes
for no reason.

Move the default controller assignment before the check.

https://bugzilla.redhat.com/show_bug.cgi?id=1168849
2015-02-25 10:04:58 +01:00
Michal Privoznik
cf2d4c603c qemu: Use correct flags for ABI stability check in SaveImageUpdateDef
https://bugzilla.redhat.com/show_bug.cgi?id=1183869

Soo. you've successfully started yourself a domain. And since you want
to use it on your host exclusively you are confident enough to
passthrough the host CPU model, like this:

  <cpu mode='host-passthrough'/>

Then, after a while, you want to save the domain into a file (e.g.
virsh save dom dom.save). And here comes the trouble. The file consist
of two parts: Libvirt header (containing domain XML among other
things), and qemu migration data. Now, the domain XML in the header is
formatted using special flags (VIR_DOMAIN_XML_SECURE |
VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_INACTIVE |
VIR_DOMAIN_XML_MIGRATABLE).

Then, on your way back from the bar, you think of changing something
in the XML in the saved file (we have a command for it after all), say
listen address for graphics console. So you successfully type in the
command:

  virsh save-image-edit dom.save

Change all the bits, and exit the editor. But instead of success
you're left with sad error message:

  error: unsupported configuration: Target CPU model <null> does not
  match source Pentium Pro

Sigh. Digging into the code you see lines, where we check for ABI
stability. The new XML you've produced is compared with the old one
from the saved file to see if qemu ABI will break or not. Wait, what?
We are using different flags to parse the XML you've provided so we
were just lucky it worked in some cases? Yep, that's right.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-02-25 09:28:54 +01:00
Michal Privoznik
0e69d97648 cpu: Format <cpu/> properly
Well, not that we are not formatting invalid XML, rather than not as
beautiful as we can:

  <cpu mode='host-passthrough'>
  </cpu>

If there are no children, let's use the singleton element.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-02-25 09:26:04 +01:00
Michal Privoznik
33912cc03e virCPUDefFormatBufFull: Use our general error handling pattern
Well, so far there are no variables to free, no cleanup work needed on
an error, so bare 'return -1;' after each error is just okay. But this
will change in a while.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-02-25 09:23:45 +01:00
Michal Privoznik
83c5467ee9 util: Introduce virBufferAddBuffer
This API joins the following two lines:

char *s = virBufferContentAndReset(buf1);
virBufferAdd(buf2, s, -1);

into one:

virBufferAddBuffer(buf2, buf1);

With one exception: there's no re-indentation applied to @buf1.
The idea is, that in general both can have different indentation
(like the test I'm adding proves)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-02-25 09:23:42 +01:00
Pavel Hrdina
efd30e2e1c qemu: fix memory leak while starting a guest
In commit cc41c648 I've re-factored qemuMonitorFindBalloonObjectPath, but
missed that there is a memory leak. The "nextpath" variable is
overwritten while looping in for cycle and we have to free it before next
cycle.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2015-02-24 16:38:50 +01:00
Michal Privoznik
39df9d2f12 network_conf: Forbid commas in DNS TXT
https://bugzilla.redhat.com/show_bug.cgi?id=1151942

While the restriction doesn't have origin in any RFC, it matters
to us while constructing the dnsmasq config file (or command line
previously). For better picture, this is how the corresponding
part of network XML look like:

  <dns>
    <forwarder addr='8.8.4.4'/>
    <txt name='example' value='example value'/>
  </dns>

And this is how the config file looks like then:

  server=8.8.4.4
  txt-record=example,example value

Now we can see why there can't be any commas in the TXT name.
They are used by dnsmasq to separate @name and @value.

Funny, we have it in the documentation, but the code (which was
pushed back in 2011) didn't reflect that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-02-24 10:07:47 +01:00
Stefan Zimmermann
8e6ee9f280 Rework s390 architecture checking
Making use of the ARCH_IS_S390 macro introduced with
e808357528

Signed-off-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
2015-02-23 14:51:02 -05:00
Stefan Zimmermann
09ab9dcc85 Prevent default creation of usb controller on s390 and s390x
Since s390 does not support usb the default creation of a usb controller
for a domain should not occur.

Also adjust s390 test cases by removing usb device instances since
usb devices are no longer created by default for s390 the s390
test cases need to be adjusted.

Signed-off-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
2015-02-23 14:50:15 -05:00