[libvirt] [PATCH v2] pci: Give an explicit error if device not found
by Cole Robinson
v2: Use intended F_OK. Drop devdir param, just check dev->path for device
existence.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/util/pci.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index 81193b7..3fcc658 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1046,6 +1046,13 @@ pciGetDevice(unsigned domain,
snprintf(dev->path, sizeof(dev->path),
PCI_SYSFS "devices/%s/config", dev->name);
+ if (access(dev->path, F_OK) != 0) {
+ pciReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Device %s not found"), dev->name);
+ pciFreeDevice(dev);
+ return NULL;
+ }
+
vendor = pciReadDeviceID(dev, "vendor");
product = pciReadDeviceID(dev, "device");
--
1.6.6.1
14 years, 7 months
[libvirt] [PATCH] build: fix cygwin build
by Eric Blake
make[3]: *** No rule to make target `-lxml2', needed by `libvirt.la'. Stop.
Due to treating the wrong string as a dependency.
* src/Makefile.am (libvirt_la_DEPENDENCIES): Depend only on
locally-built file, not on strings that might resolve as '-lxml2'.
---
Pushing under the obvious rule, since this was breaking a build.
src/Makefile.am | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 15bc8fc..72f23a7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -974,13 +974,13 @@ libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \
$(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS)
libvirt_la_LIBADD += $(LIBXML_LIBS) \
$(LIBPCAP_LIBS) \
$(DRIVER_MODULE_LIBS) \
$(CYGWIN_EXTRA_LIBADD) ../gnulib/lib/libgnu.la
libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT
-libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) $(LIBVIRT_SYMBOL_FILE)
+libvirt_la_DEPENDENCIES = ../gnulib/lib/libgnu.la $(LIBVIRT_SYMBOL_FILE)
# Create an automake "convenience library" version of libvirt_la,
# just for testing, since the test harness requires access to internal
# bits and pieces that we don't want to make publicly accessible.
noinst_LTLIBRARIES += libvirt_test.la
--
1.7.0.1
14 years, 7 months
[libvirt] [PATCH] Protect against NULL pointer flaws in monitor usage (v2)
by Daniel P. Berrange
History has shown that there are frequent bugs in the QEMU driver
code leading to the monitor being invoked with a NULL pointer.
Although the QEMU driver code should always report an error in
this case before invoking the monitor, as a safety net put in a
generic check in the monitor code entry points.
* src/qemu/qemu_monitor.c: Safety net to check for NULL monitor
object
---
src/qemu/qemu_monitor.c | 436 +++++++++++++++++++++++++++++++++++++++--------
1 files changed, 368 insertions(+), 68 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index efaf74a..ec0c3fe 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -891,7 +891,13 @@ int qemuMonitorEmitGraphics(qemuMonitorPtr mon,
int qemuMonitorSetCapabilities(qemuMonitorPtr mon)
{
int ret;
- DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSetCapabilities(mon);
@@ -906,7 +912,13 @@ qemuMonitorStartCPUs(qemuMonitorPtr mon,
virConnectPtr conn)
{
int ret;
- DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONStartCPUs(mon, conn);
@@ -920,7 +932,13 @@ int
qemuMonitorStopCPUs(qemuMonitorPtr mon)
{
int ret;
- DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONStopCPUs(mon);
@@ -933,7 +951,13 @@ qemuMonitorStopCPUs(qemuMonitorPtr mon)
int qemuMonitorSystemPowerdown(qemuMonitorPtr mon)
{
int ret;
- DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSystemPowerdown(mon);
@@ -947,7 +971,13 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
int **pids)
{
int ret;
- DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONGetCPUInfo(mon, pids);
@@ -960,7 +990,13 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
unsigned long *currmem)
{
int ret;
- DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONGetBalloonInfo(mon, currmem);
@@ -975,7 +1011,13 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
unsigned int nr_stats)
{
int ret;
- DEBUG("mon=%p, fd=%d stats=%p nstats=%u", mon, mon->fd, stats, nr_stats);
+ DEBUG("mon=%p stats=%p nstats=%u", mon, stats, nr_stats);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONGetMemoryStats(mon, stats, nr_stats);
@@ -994,7 +1036,13 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
long long *errs)
{
int ret;
- DEBUG("mon=%p, fd=%d dev=%s", mon, mon->fd, devname);
+ DEBUG("mon=%p dev=%s", mon, devname);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONGetBlockStatsInfo(mon, devname,
@@ -1014,8 +1062,14 @@ int qemuMonitorGetBlockExtent(qemuMonitorPtr mon,
unsigned long long *extent)
{
int ret;
- DEBUG("mon=%p, fd=%d, devname=%p",
- mon, mon->fd, devname);
+ DEBUG("mon=%p, devname=%p",
+ mon, devname);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONGetBlockExtent(mon, devname, extent);
@@ -1030,8 +1084,14 @@ int qemuMonitorSetVNCPassword(qemuMonitorPtr mon,
const char *password)
{
int ret;
- DEBUG("mon=%p, fd=%d, password=%p",
- mon, mon->fd, password);
+ DEBUG("mon=%p, password=%p",
+ mon, password);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (!password)
password = "";
@@ -1052,8 +1112,14 @@ int qemuMonitorSetGraphicsPassword(qemuMonitorPtr mon,
unsigned int expiry)
{
int ret;
- DEBUG("mon=%p, fd=%d type=%d, password=%p, expiry=%u",
- mon, mon->fd, type, password, expiry);
+ DEBUG("mon=%p type=%d, password=%p, expiry=%u",
+ mon, type, password, expiry);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (!password)
password = "";
@@ -1071,7 +1137,13 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon,
unsigned long newmem)
{
int ret;
- DEBUG("mon=%p, fd=%d newmem=%lu", mon, mon->fd, newmem);
+ DEBUG("mon=%p newmem=%lu", mon, newmem);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSetBalloon(mon, newmem);
@@ -1084,7 +1156,13 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon,
int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, int online)
{
int ret;
- DEBUG("mon=%p, fd=%d cpu=%d online=%d", mon, mon->fd, cpu, online);
+ DEBUG("mon=%p cpu=%d online=%d", mon, cpu, online);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSetCPU(mon, cpu, online);
@@ -1098,7 +1176,13 @@ int qemuMonitorEjectMedia(qemuMonitorPtr mon,
const char *devname)
{
int ret;
- DEBUG("mon=%p, fd=%d devname=%s", mon, mon->fd, devname);
+ DEBUG("mon=%p devname=%s", mon, devname);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONEjectMedia(mon, devname);
@@ -1114,8 +1198,14 @@ int qemuMonitorChangeMedia(qemuMonitorPtr mon,
const char *format)
{
int ret;
- DEBUG("mon=%p, fd=%d devname=%s newmedia=%s format=%s",
- mon, mon->fd, devname, newmedia, format);
+ DEBUG("mon=%p devname=%s newmedia=%s format=%s",
+ mon, devname, newmedia, format);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONChangeMedia(mon, devname, newmedia, format);
@@ -1131,8 +1221,14 @@ int qemuMonitorSaveVirtualMemory(qemuMonitorPtr mon,
const char *path)
{
int ret;
- DEBUG("mon=%p, fd=%d offset=%llu length=%zu path=%s",
- mon, mon->fd, offset, length, path);
+ DEBUG("mon=%p offset=%llu length=%zu path=%s",
+ mon, offset, length, path);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSaveVirtualMemory(mon, offset, length, path);
@@ -1147,8 +1243,14 @@ int qemuMonitorSavePhysicalMemory(qemuMonitorPtr mon,
const char *path)
{
int ret;
- DEBUG("mon=%p, fd=%d offset=%llu length=%zu path=%s",
- mon, mon->fd, offset, length, path);
+ DEBUG("mon=%p offset=%llu length=%zu path=%s",
+ mon, offset, length, path);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSavePhysicalMemory(mon, offset, length, path);
@@ -1162,7 +1264,13 @@ int qemuMonitorSetMigrationSpeed(qemuMonitorPtr mon,
unsigned long bandwidth)
{
int ret;
- DEBUG("mon=%p, fd=%d bandwidth=%lu", mon, mon->fd, bandwidth);
+ DEBUG("mon=%p bandwidth=%lu", mon, bandwidth);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSetMigrationSpeed(mon, bandwidth);
@@ -1176,7 +1284,13 @@ int qemuMonitorSetMigrationDowntime(qemuMonitorPtr mon,
unsigned long long downtime)
{
int ret;
- DEBUG("mon=%p, fd=%d downtime=%llu", mon, mon->fd, downtime);
+ DEBUG("mon=%p downtime=%llu", mon, downtime);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSetMigrationDowntime(mon, downtime);
@@ -1193,7 +1307,13 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon,
unsigned long long *total)
{
int ret;
- DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONGetMigrationStatus(mon, status,
@@ -1215,8 +1335,14 @@ int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
int port)
{
int ret;
- DEBUG("mon=%p, fd=%d hostname=%s port=%d",
- mon, mon->fd, hostname, port);
+ DEBUG("mon=%p hostname=%s port=%d",
+ mon, hostname, port);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONMigrateToHost(mon, background, hostname, port);
@@ -1231,8 +1357,14 @@ int qemuMonitorMigrateToCommand(qemuMonitorPtr mon,
const char * const *argv)
{
int ret;
- DEBUG("mon=%p, fd=%d argv=%p",
- mon, mon->fd, argv);
+ DEBUG("mon=%p argv=%p",
+ mon, argv);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONMigrateToCommand(mon, background, argv);
@@ -1248,8 +1380,14 @@ int qemuMonitorMigrateToFile(qemuMonitorPtr mon,
unsigned long long offset)
{
int ret;
- DEBUG("mon=%p, fd=%d argv=%p target=%s offset=%llu",
- mon, mon->fd, argv, target, offset);
+ DEBUG("mon=%p argv=%p target=%s offset=%llu",
+ mon, argv, target, offset);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (offset % QEMU_MONITOR_MIGRATE_TO_FILE_BS) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1270,8 +1408,14 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
const char *unixfile)
{
int ret;
- DEBUG("mon=%p fd=%d unixfile=%s",
- mon, mon->fd, unixfile);
+ DEBUG("mon=%p, unixfile=%s",
+ mon, unixfile);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONMigrateToUnix(mon, background, unixfile);
@@ -1283,7 +1427,13 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
{
int ret;
- DEBUG("mon=%p fd=%d", mon, mon->fd);
+ DEBUG("mon=%p", mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONMigrateCancel(mon);
@@ -1304,6 +1454,12 @@ int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
DEBUG("mon=%p type=%d hostname=%s port=%d tlsPort=%d tlsSubject=%s",
mon, type, hostname, port, tlsPort, NULLSTR(tlsSubject));
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONGraphicsRelocate(mon,
type,
@@ -1327,7 +1483,13 @@ int qemuMonitorAddUSBDisk(qemuMonitorPtr mon,
const char *path)
{
int ret;
- DEBUG("mon=%p, fd=%d path=%s", mon, mon->fd, path);
+ DEBUG("mon=%p path=%s", mon, path);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONAddUSBDisk(mon, path);
@@ -1342,7 +1504,13 @@ int qemuMonitorAddUSBDeviceExact(qemuMonitorPtr mon,
int dev)
{
int ret;
- DEBUG("mon=%p, fd=%d bus=%d dev=%d", mon, mon->fd, bus, dev);
+ DEBUG("mon=%p bus=%d dev=%d", mon, bus, dev);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONAddUSBDeviceExact(mon, bus, dev);
@@ -1356,8 +1524,14 @@ int qemuMonitorAddUSBDeviceMatch(qemuMonitorPtr mon,
int product)
{
int ret;
- DEBUG("mon=%p, fd=%d vendor=%d product=%d",
- mon, mon->fd, vendor, product);
+ DEBUG("mon=%p vendor=%d product=%d",
+ mon, vendor, product);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONAddUSBDeviceMatch(mon, vendor, product);
@@ -1372,10 +1546,16 @@ int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon,
virDomainDevicePCIAddress *guestAddr)
{
int ret;
- DEBUG("mon=%p, fd=%d domain=%d bus=%d slot=%d function=%d",
- mon, mon->fd,
+ DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d",
+ mon,
hostAddr->domain, hostAddr->bus, hostAddr->slot, hostAddr->function);
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONAddPCIHostDevice(mon, hostAddr, guestAddr);
else
@@ -1390,8 +1570,14 @@ int qemuMonitorAddPCIDisk(qemuMonitorPtr mon,
virDomainDevicePCIAddress *guestAddr)
{
int ret;
- DEBUG("mon=%p, fd=%d path=%s bus=%s",
- mon, mon->fd, path, bus);
+ DEBUG("mon=%p path=%s bus=%s",
+ mon, path, bus);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONAddPCIDisk(mon, path, bus, guestAddr);
@@ -1406,7 +1592,13 @@ int qemuMonitorAddPCINetwork(qemuMonitorPtr mon,
virDomainDevicePCIAddress *guestAddr)
{
int ret;
- DEBUG("mon=%p, fd=%d nicstr=%s", mon, mon->fd, nicstr);
+ DEBUG("mon=%p nicstr=%s", mon, nicstr);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONAddPCINetwork(mon, nicstr, guestAddr);
@@ -1420,10 +1612,16 @@ int qemuMonitorRemovePCIDevice(qemuMonitorPtr mon,
virDomainDevicePCIAddress *guestAddr)
{
int ret;
- DEBUG("mon=%p, fd=%d domain=%d bus=%d slot=%d function=%d",
- mon, mon->fd, guestAddr->domain, guestAddr->bus,
+ DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d",
+ mon, guestAddr->domain, guestAddr->bus,
guestAddr->slot, guestAddr->function);
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONRemovePCIDevice(mon, guestAddr);
else
@@ -1437,8 +1635,14 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
int fd)
{
int ret;
- DEBUG("mon=%p, fd=%d fdname=%s fd=%d",
- mon, mon->fd, fdname, fd);
+ DEBUG("mon=%p, fdname=%s fd=%d",
+ mon, fdname, fd);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONSendFileHandle(mon, fdname, fd);
@@ -1452,8 +1656,14 @@ int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
const char *fdname)
{
int ret;
- DEBUG("mon=%p, fd=%d fdname=%s",
- mon, mon->fd, fdname);
+ DEBUG("mon=%p fdname=%s",
+ mon, fdname);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONCloseFileHandle(mon, fdname);
@@ -1467,8 +1677,14 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon,
const char *netstr)
{
int ret;
- DEBUG("mon=%p, fd=%d netstr=%s",
- mon, mon->fd, netstr);
+ DEBUG("mon=%p netstr=%s",
+ mon, netstr);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONAddHostNetwork(mon, netstr);
@@ -1483,8 +1699,14 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon,
const char *netname)
{
int ret;
- DEBUG("mon=%p, fd=%d netname=%s",
- mon, mon->fd, netname);
+ DEBUG("mon=%p netname=%s",
+ mon, netname);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONRemoveHostNetwork(mon, vlan, netname);
@@ -1498,8 +1720,14 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon,
const char *netdevstr)
{
int ret;
- DEBUG("mon=%p, fd=%d netdevstr=%s",
- mon, mon->fd, netdevstr);
+ DEBUG("mon=%p netdevstr=%s",
+ mon, netdevstr);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONAddNetdev(mon, netdevstr);
@@ -1513,8 +1741,14 @@ int qemuMonitorRemoveNetdev(qemuMonitorPtr mon,
const char *alias)
{
int ret;
- DEBUG("mon=%p, fd=%d alias=%s",
- mon, mon->fd, alias);
+ DEBUG("mon=%p alias=%s",
+ mon, alias);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONRemoveNetdev(mon, alias);
@@ -1528,8 +1762,14 @@ int qemuMonitorGetPtyPaths(qemuMonitorPtr mon,
virHashTablePtr paths)
{
int ret;
- DEBUG("mon=%p, fd=%d",
- mon, mon->fd);
+ DEBUG("mon=%p",
+ mon);
+
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
if (mon->json)
ret = qemuMonitorJSONGetPtyPaths(mon, paths);
@@ -1543,9 +1783,15 @@ int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon,
const char *bus,
virDomainDevicePCIAddress *guestAddr)
{
- DEBUG("mon=%p, fd=%d type=%s", mon, mon->fd, bus);
+ DEBUG("mon=%p type=%s", mon, bus);
int ret;
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONAttachPCIDiskController(mon, bus, guestAddr);
else
@@ -1560,12 +1806,18 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon,
virDomainDevicePCIAddress *controllerAddr,
virDomainDeviceDriveAddress *driveAddr)
{
- DEBUG("mon=%p, fd=%d drivestr=%s domain=%d bus=%d slot=%d function=%d",
- mon, mon->fd, drivestr,
+ DEBUG("mon=%p drivestr=%s domain=%d bus=%d slot=%d function=%d",
+ mon, drivestr,
controllerAddr->domain, controllerAddr->bus,
controllerAddr->slot, controllerAddr->function);
int ret;
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONAttachDrive(mon, drivestr, controllerAddr, driveAddr);
else
@@ -1577,9 +1829,15 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon,
int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
qemuMonitorPCIAddress **addrs)
{
- DEBUG("mon=%p, fd=%d addrs=%p", mon, mon->fd, addrs);
+ DEBUG("mon=%p addrs=%p", mon, addrs);
int ret;
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONGetAllPCIAddresses(mon, addrs);
else
@@ -1590,9 +1848,15 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
int qemuMonitorDelDevice(qemuMonitorPtr mon,
const char *devalias)
{
- DEBUG("mon=%p, fd=%d devalias=%s", mon, mon->fd, devalias);
+ DEBUG("mon=%p devalias=%s", mon, devalias);
int ret;
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONDelDevice(mon, devalias);
else
@@ -1604,9 +1868,15 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon,
int qemuMonitorAddDevice(qemuMonitorPtr mon,
const char *devicestr)
{
- DEBUG("mon=%p, fd=%d device=%s", mon, mon->fd, devicestr);
+ DEBUG("mon=%p device=%s", mon, devicestr);
int ret;
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONAddDevice(mon, devicestr);
else
@@ -1617,9 +1887,15 @@ int qemuMonitorAddDevice(qemuMonitorPtr mon,
int qemuMonitorAddDrive(qemuMonitorPtr mon,
const char *drivestr)
{
- DEBUG("mon=%p, fd=%d drive=%s", mon, mon->fd, drivestr);
+ DEBUG("mon=%p drive=%s", mon, drivestr);
int ret;
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONAddDrive(mon, drivestr);
else
@@ -1632,9 +1908,15 @@ int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon,
const char *alias,
const char *passphrase)
{
- DEBUG("mon=%p, fd=%d alias=%s passphrase=%p(value hidden)", mon, mon->fd, alias, passphrase);
+ DEBUG("mon=%p alias=%s passphrase=%p(value hidden)", mon, alias, passphrase);
int ret;
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONSetDrivePassphrase(mon, alias, passphrase);
else
@@ -1648,6 +1930,12 @@ int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name)
DEBUG("mon=%p, name=%s",mon,name);
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONCreateSnapshot(mon, name);
else
@@ -1661,6 +1949,12 @@ int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name)
DEBUG("mon=%p, name=%s",mon,name);
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONLoadSnapshot(mon, name);
else
@@ -1674,6 +1968,12 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name)
DEBUG("mon=%p, name=%s",mon,name);
+ if (!mon) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
if (mon->json)
ret = qemuMonitorJSONDeleteSnapshot(mon, name);
else
--
1.6.6.1
14 years, 7 months
[libvirt] [PATCH] pci: Give an explicit error if device not found
by Cole Robinson
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/util/pci.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index 81193b7..e1e11bc 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1028,6 +1028,7 @@ pciGetDevice(unsigned domain,
unsigned function)
{
pciDevice *dev;
+ char devdir[PATH_MAX];
char *vendor, *product;
if (VIR_ALLOC(dev) < 0) {
@@ -1043,8 +1044,17 @@ pciGetDevice(unsigned domain,
snprintf(dev->name, sizeof(dev->name), "%.4x:%.2x:%.2x.%.1x",
dev->domain, dev->bus, dev->slot, dev->function);
+ snprintf(devdir, sizeof(devdir),
+ PCI_SYSFS "devices/%s", dev->name);
snprintf(dev->path, sizeof(dev->path),
- PCI_SYSFS "devices/%s/config", dev->name);
+ "%s/%s/config", devdir, dev->name);
+
+ if (access(devdir, X_OK) != 0) {
+ pciReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Device %s not found"), dev->name);
+ pciFreeDevice(dev);
+ return NULL;
+ }
vendor = pciReadDeviceID(dev, "vendor");
product = pciReadDeviceID(dev, "device");
--
1.7.0.1
14 years, 7 months
[libvirt] [PATCH] umlAutostartDomain: avoid NULL-deref upon virGetLastError failure
by Jim Meyering
Another...
>From 09579adb73f04c5b940ad28cdfc242cff0726e0e Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 17 May 2010 14:38:14 +0200
Subject: [PATCH] umlAutostartDomain: avoid NULL-deref upon virGetLastError failure
* src/uml/uml_driver.c (umlAutostartDomain): Handle a NULL return
from virGetLastError.
---
src/uml/uml_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 63fe808..ffb87c8 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -157,7 +157,7 @@ umlAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaqu
if (umlStartVMDaemon(data->conn, data->driver, vm) < 0) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to autostart VM '%s': %s"),
- vm->def->name, err->message);
+ vm->def->name, err ? err->message : "");
}
}
virDomainObjUnlock(vm);
--
1.7.1.250.g7d1e8
14 years, 7 months
[libvirt] [PATCH] build: fix up some compiler flags
by Eric Blake
Matthias noted that the line:
virt_aa_helper_LDFLAGS = $(WARN_CFLAGS)
looks inconsistent, so I did an audit.
Currently, the set of compiler warning flags passed to gcc as $CC are
equally permitted as the set of linker flags passed to gcc as $LD, so
there was no problem with that usage. But if we ever get in a
situation where $CC and $LD treat particular flags differently, using
the right variable form will make it easier.
In the process, I spotted a couple of typos that were omitting useful
flags, as well as specifying a -l under the wrong variable.
* acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Define WARN_LDFLAGS as
an alias for WARN_CFLAGS.
* tools/Makefile.am (virsh_LDFLAGS): Use more canonical spelling.
* proxy/Makefile.am (libvirt_proxy_LDFLAGS): Likewise. Move
library...
(libvirt_proxy_LDADD): ...here.
* src/Makefile.am (virt_aa_helper_LDFLAGS): Use more canonical
spelling of WARN_LDFLAGS.
(libvirt_parthelper_LDFLAGS, libvirt_lxc_LDFLAGS): Likewise. Use
correct spelling of COVERAGE_LDFLAGS.
Reported by Matthias Bolte.
---
acinclude.m4 | 3 ++-
proxy/Makefile.am | 4 ++--
src/Makefile.am | 6 +++---
tools/Makefile.am | 4 ++--
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index 7fa2d67..f048879 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -78,7 +78,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
AC_MSG_RESULT($complCFLAGS)
WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS"
- AC_SUBST(WARN_CFLAGS)
+ AC_SUBST([WARN_CFLAGS])
+ AC_SUBST([WARN_LDFLAGS])
dnl Needed to keep compile quiet on python 2.4
COMPILER_FLAGS=
diff --git a/proxy/Makefile.am b/proxy/Makefile.am
index bee47d0..4716683 100644
--- a/proxy/Makefile.am
+++ b/proxy/Makefile.am
@@ -32,9 +32,9 @@ libvirt_proxy_SOURCES = libvirt_proxy.c \
@top_srcdir(a)/src/xen/xen_hypervisor.c \
@top_srcdir(a)/src/xen/sexpr.c \
@top_srcdir(a)/src/xen/xs_internal.c
-libvirt_proxy_LDFLAGS = $(WARN_CFLAGS) $(XEN_LIBS)
+libvirt_proxy_LDFLAGS = $(WARN_LDFLAGS)
libvirt_proxy_DEPENDENCIES =
-libvirt_proxy_LDADD = ../gnulib/lib/libgnu.la $(LIB_PTHREAD)
+libvirt_proxy_LDADD = ../gnulib/lib/libgnu.la $(XEN_LIBS) $(LIB_PTHREAD)
install-exec-hook:
chmod u+s $(DESTDIR)$(libexecdir)/libvirt_proxy
diff --git a/src/Makefile.am b/src/Makefile.am
index 72f23a7..034feec 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1004,7 +1004,7 @@ if WITH_LIBVIRTD
libexec_PROGRAMS += libvirt_parthelper
libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES)
-libvirt_parthelper_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS)
+libvirt_parthelper_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS)
libvirt_parthelper_LDADD = $(LIBPARTED_LIBS)
libvirt_parthelper_CFLAGS = $(LIBPARTED_CFLAGS)
endif
@@ -1024,7 +1024,7 @@ libvirt_lxc_SOURCES = \
$(DOMAIN_CONF_SOURCES) \
$(CPU_CONF_SOURCES) \
$(NWFILTER_PARAM_CONF_SOURCES)
-libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS)
+libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS)
libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
$(LIBXML_LIBS) $(NUMACTL_LIBS) $(LIB_PTHREAD) \
../gnulib/lib/libgnu.la
@@ -1044,7 +1044,7 @@ libexec_PROGRAMS += virt-aa-helper
virt_aa_helper_SOURCES = $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES)
-virt_aa_helper_LDFLAGS = $(WARN_CFLAGS)
+virt_aa_helper_LDFLAGS = $(WARN_LDFLAGS)
virt_aa_helper_LDADD = \
$(LIBXML_LIBS) \
libvirt_conf.la \
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 33a3216..fd05e8b 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -34,14 +34,14 @@ virsh_SOURCES = \
console.c console.h \
virsh.c
-virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS)
+virsh_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS)
virsh_LDADD = \
$(STATIC_BINARIES) \
$(WARN_CFLAGS) \
../src/libvirt.la \
../gnulib/lib/libgnu.la \
$(VIRSH_LIBS)
-virsh_CFLAGS = \
+virsh_CFLAGS = \
-I$(top_srcdir)/gnulib/lib -I../gnulib/lib \
-I../include -I$(top_srcdir)/include \
-I$(top_srcdir)/src \
--
1.7.0.1
14 years, 7 months
[libvirt] [PATCH] build: avoid compile failure on linux kernels older than 2.6.19
by Jim Meyering
This is required for any kernel prior to 2.6.19,
since <linux/magic.h> didn't exist back then.
Now that file is provided by the kernel-headers package.
>From d14ef1669968ffeb65076b007e318934ed99aa61 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 17 May 2010 16:17:08 +0200
Subject: [PATCH] build: avoid compile failure on linux kernels older than 2.6.19
* configure.ac: Check for <linux/magic.h>.
* src/util/storage_file.c: Include <linux/magic.h> only if present.
Linux kernels prior to 2.6.19 lacked it.
[__linux__] (NFS_SUPER_MAGIC): Define if not already defined.
---
configure.ac | 2 +-
src/util/storage_file.c | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index c187420..ebd2082 100644
--- a/configure.ac
+++ b/configure.ac
@@ -108,7 +108,7 @@ LIBS=$old_libs
dnl Availability of various common headers (non-fatal if missing).
AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \
- termios.h sys/poll.h syslog.h mntent.h net/ethernet.h])
+ termios.h sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h])
dnl Where are the XDR functions?
dnl If portablexdr is installed, prefer that.
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 5f15a64..a07bedc 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -27,7 +27,9 @@
#include <unistd.h>
#include <fcntl.h>
#ifdef __linux__
-# include <linux/magic.h>
+# if HAVE_LINUX_MAGIC_H
+# include <linux/magic.h>
+# endif
# include <sys/statfs.h>
#endif
#include "dirname.h"
@@ -416,6 +418,9 @@ virStorageFileGetMetadata(const char *path,
#ifdef __linux__
+# ifndef NFS_SUPER_MAGIC
+# define NFS_SUPER_MAGIC 0x6969
+# endif
# ifndef OCFS2_SUPER_MAGIC
# define OCFS2_SUPER_MAGIC 0x7461636f
# endif
--
1.7.1.250.g7d1e8
14 years, 7 months
[libvirt] [PATCH] x86ModelHasFeature: avoid NULL-dereference for unmatched CPU "feature"
by Jim Meyering
Here's another fix for a potential NULL-deref.
x86cpuidFind can return NULL, yet this caller
would dereference that pointer (via x86cpuidMatchMasked)
without first checking.
>From 9e759e2714b67ea98b18aafb66b5a99ad6361086 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 17 May 2010 14:06:13 +0200
Subject: [PATCH] x86ModelHasFeature: avoid NULL-dereference for unmatched CPU "feature"
* src/cpu/cpu_x86.c (x86ModelHasFeature): Do not dereference the pointer
returned by x86cpuidFind without first ensuring it is non-NULL.
---
src/cpu/cpu_x86.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 633eb69..f7473bf 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -602,31 +602,31 @@ x86ModelMergeFeature(struct x86_model *model,
static bool
x86ModelHasFeature(struct x86_model *model,
const struct x86_feature *feature)
{
unsigned int i;
struct cpuX86cpuid *cpuid;
struct cpuX86cpuid *model_cpuid;
if (feature == NULL)
return false;
for (i = 0; i < feature->ncpuid; i++) {
cpuid = feature->cpuid + i;
model_cpuid = x86cpuidFind(model->cpuid, model->ncpuid,
cpuid->function);
- if (!x86cpuidMatchMasked(model_cpuid, cpuid))
+ if (!model_cpuid || !x86cpuidMatchMasked(model_cpuid, cpuid))
return false;
}
return true;
}
static struct x86_model *
x86ModelFromCPU(const virCPUDefPtr cpu,
const struct x86_map *map,
int policy)
{
struct x86_model *model = NULL;
int i;
--
1.7.1.250.g7d1e8
14 years, 7 months
[libvirt] [PATCH] qemu: Report cmdline output if VM dies early
by Cole Robinson
qemuReadLogOutput early VM death detection is racy and won't always work.
Startup then errors when connecting to the VM monitor. This won't report
the emulator cmdline output which is typically the most useful diagnostic.
Check if the VM has died at the very end of the monitor connection step,
and if so, report the cmdline output.
See also: https://bugzilla.redhat.com/show_bug.cgi?id=581381
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_driver.c | 59 ++++++++++++++++++++++++++++++++++-------------
1 files changed, 42 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 42a653a..c446028 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2029,39 +2029,47 @@ static void qemudFreePtyPath(void *payload, const char *name ATTRIBUTE_UNUSED)
VIR_FREE(payload);
}
+static void
+qemuReadLogFD(int logfd, char *buf, int maxlen, int off)
+{
+ int ret;
+ char *tmpbuf = buf+off;
+
+ ret = saferead(logfd, tmpbuf, maxlen-off-1);
+ if (ret < 0) {
+ ret = 0;
+ }
+
+ *(tmpbuf+ret) = '\0';
+}
+
static int
qemudWaitForMonitor(struct qemud_driver* driver,
virDomainObjPtr vm, off_t pos)
{
- char buf[4096]; /* Plenty of space to get startup greeting */
+ char buf[4096] = "\0"; /* Plenty of space to get startup greeting */
int logfd;
int ret = -1;
+ virHashTablePtr paths = NULL;
- if ((logfd = qemudLogReadFD(driver->logDir, vm->def->name, pos))
- < 0)
+ if ((logfd = qemudLogReadFD(driver->logDir, vm->def->name, pos)) < 0)
return -1;
- ret = qemudReadLogOutput(vm, logfd, buf, sizeof(buf),
- qemudFindCharDevicePTYs,
- "console", 30);
- if (close(logfd) < 0) {
- char ebuf[4096];
- VIR_WARN(_("Unable to close logfile: %s"),
- virStrerror(errno, ebuf, sizeof ebuf));
- }
-
- if (ret < 0)
- return -1;
+ if (qemudReadLogOutput(vm, logfd, buf, sizeof(buf),
+ qemudFindCharDevicePTYs,
+ "console", 30) < 0)
+ goto closelog;
VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name);
- if (qemuConnectMonitor(driver, vm) < 0)
- return -1;
+ if (qemuConnectMonitor(driver, vm) < 0) {
+ goto cleanup;
+ }
/* Try to get the pty path mappings again via the monitor. This is much more
* reliable if it's available.
* Note that the monitor itself can be on a pty, so we still need to try the
* log output method. */
- virHashTablePtr paths = virHashCreate(0);
+ paths = virHashCreate(0);
if (paths == NULL) {
virReportOOMError();
goto cleanup;
@@ -2082,6 +2090,23 @@ cleanup:
virHashFree(paths, qemudFreePtyPath);
}
+ if (kill(vm->pid, 0) == -1 && errno == ESRCH) {
+ /* VM is dead, any other error raised in the interim is probably
+ * not as important as the qemu cmdline output */
+ qemuReadLogFD(logfd, buf, sizeof(buf), strlen(buf));
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("process exited while connecting to monitor: %s"),
+ buf);
+ ret = -1;
+ }
+
+closelog:
+ if (close(logfd) < 0) {
+ char ebuf[4096];
+ VIR_WARN(_("Unable to close logfile: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
+
return ret;
}
--
1.7.0.1
14 years, 7 months
[libvirt] [PATCH] qemu_driver: avoid NULL dereference
by Jim Meyering
The following theoretical possibility of a NULL dereference
has been in the code since April 1
(commit 6e41f30efcac08e50b21d9c943d6d27e90555951).
It's theoretical, because if that vm = NULL
statement is ever executed, the very next one,
calling virDomainObjUnlock would dereference that now-NULL "vm".
Hence, I think we can conclude the vm = NULL statement is
effectively dead code. That conclusion is in line with the
"should" in the preceding comment.
At first, it seemed like it would deserve an sa_assert.
But without the assert, "n_refs" would be unused,
(so this first patch is solely FYI -- not proposed)
I solved it differently in the 2nd patch.
>From 524aec3ebed613f86b64584d2f461f4a18d2e618 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 17 May 2010 12:10:52 +0200
Subject: [PATCH] qemu_driver: avoid NULL dereference
* src/qemu/qemu_driver.c (qemudDomainStart): Rather than trying to
handle a "can't happen" case, simply sa_assert that it won't happen.
---
src/qemu/qemu_driver.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8f69b5a..819ea17 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6489,8 +6489,9 @@ static int qemudDomainStart(virDomainPtr dom) {
* We should still have a reference left to vm but
* one should check for 0 anyway
*/
- if (qemuDomainObjEndJob(vm) == 0)
- vm = NULL;
+ int n_refs = qemuDomainObjEndJob(vm);
+ sa_assert (n_refs);
+
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
ret = qemudDomainRestore(dom->conn, managed_save);
--
1.7.1.250.g7d1e8
>From f88969b986a1c88985671c9d6fa9cb1dc449ed74 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 17 May 2010 12:10:52 +0200
Subject: [PATCH] qemu_driver: avoid NULL dereference
* src/qemu/qemu_driver.c (qemudDomainStart): After setting vm to NULL,
goto cleanup, rather than dereferencing the NULL pointer.
---
src/qemu/qemu_driver.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8f69b5a..3559e36 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6489,8 +6489,11 @@ static int qemudDomainStart(virDomainPtr dom) {
* We should still have a reference left to vm but
* one should check for 0 anyway
*/
- if (qemuDomainObjEndJob(vm) == 0)
+ if (qemuDomainObjEndJob(vm) = 0) {
vm = NULL;
+ goto cleanup;
+ }
+
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
ret = qemudDomainRestore(dom->conn, managed_save);
--
1.7.1.250.g7d1e8
14 years, 7 months