From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] device-assignment: Register as un-migratable Date: Mon, 15 Nov 2010 23:42:14 +0100 Message-ID: <4CE1B746.7060606@web.de> References: <20101115194103.15991.41060.stgit@s20.home> <4CE1928F.7060100@web.de> <1289852706.2805.223.camel@x201> <4CE1AE58.3070508@web.de> <1289859927.2805.244.camel@x201> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig8E0D5AF924FE652CBBC51E2F" Cc: kvm@vger.kernel.org, mst@redhat.com, Juan Quintela To: Alex Williamson Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:34370 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933871Ab0KOWmV (ORCPT ); Mon, 15 Nov 2010 17:42:21 -0500 In-Reply-To: <1289859927.2805.244.camel@x201> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig8E0D5AF924FE652CBBC51E2F Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Am 15.11.2010 23:25, Alex Williamson wrote: > On Mon, 2010-11-15 at 23:04 +0100, Jan Kiszka wrote: >> Am 15.11.2010 21:25, Alex Williamson wrote: >>> On Mon, 2010-11-15 at 21:05 +0100, Jan Kiszka wrote: >>>> Am 15.11.2010 20:41, Alex Williamson wrote: >>>>> Use register_device_unmigratable() to declare ourselves as >>>>> non-migratable. >>>>> >>>>> Signed-off-by: Alex Williamson >>>>> --- >>>>> >>>>> hw/device-assignment.c | 15 +++++++++++++++ >>>>> 1 files changed, 15 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c >>>>> index bde231d..cd93941 100644 >>>>> --- a/hw/device-assignment.c >>>>> +++ b/hw/device-assignment.c >>>>> @@ -1434,6 +1434,13 @@ static void assigned_dev_unregister_msix_mmi= o(AssignedDevice *dev) >>>>> dev->msix_table_page =3D NULL; >>>>> } >>>>> =20 >>>>> +/* This should never get called, but we're required to create a sa= ve_state >>>>> + * handler or else the no_migrate flag will never be checked. */ >>>>> +static void assigned_save(QEMUFile* f, void *opaque) >>>>> +{ >>>>> + abort(); >>>>> +} >>>>> + >>>>> static int assigned_initfn(struct PCIDevice *pci_dev) >>>>> { >>>>> AssignedDevice *dev =3D DO_UPCAST(AssignedDevice, dev, pci_dev= ); >>>>> @@ -1490,6 +1497,13 @@ static int assigned_initfn(struct PCIDevice = *pci_dev) >>>>> =20 >>>>> assigned_dev_load_option_rom(dev); >>>>> QLIST_INSERT_HEAD(&devs, dev, next); >>>>> + >>>>> + /* Assigned devices are not migratable, register a save >>>>> + * state entry so that we can mark it unmigratable. */ >>>>> + register_savevm(&dev->dev.qdev, "pci-assign", 0, 0, >>>>> + assigned_save, NULL, dev); >>>>> + register_device_unmigratable(&dev->dev.qdev, "pci-assign", dev= ); >>>>> + >>>> >>>> Isn't this expressible via some VMStateDescription? If not, that sho= uld >>>> be changed first. >>> >>> Nope, save state handlers aren't allowed to fail. I tried to fix it:= >>> >>> http://lists.nongnu.org/archive/html/qemu-devel/2010-11/msg00417.html= >>> >>> (you can find more discussion in other branches of that subject) I'v= e >>> succumbed to not getting that series in, so now I'm just trying to us= e >>> the code as it exists. Thanks, >> >> Hmm, didn't get why you need that series for the purpose of no_migrati= on >> declaration. >> My point is: >=20 > I was hoping you were going down the path I started, that we don't need= > to special case non-migratable devices if we just allow save to return > an error. Ah, of course. Still, vmstate is the way to go IMO. >=20 >> My point is: >> >> struct VMStateDescription { >> const char *name; >> int version_id; >> int minimum_version_id; >> int minimum_version_id_old; >> int no_migrate; /* or 'flags' */ >> ... >> >> so that you can specify an empty vmstate with that flag set, and you d= o >> not need to register/unregister things via to-be-deprecated service >> calls. Or am I missing some subtle detail? >=20 > We don't seem to be enforcing that new drivers should use vmsd vs the > old style handling Don't we? We are enforcing qdev but not vmstate? Sounds silly. I think new devices should all be expressible this way, no? > and there are some drivers that are currently too > complicated for vmsd to handle, which means no_migrate has to be > registered on the save state entry, not the vmsd. So I could create a > dummy vmsd, then call register_device_unmigratable, to achieve roughly > the same effect. Six of one, half dozen of the other... I can switch t= o > a vmsd dummy save if it's preferred. Thanks, IMHO, new designs should not use register_savevm anymore. Jan --------------enig8E0D5AF924FE652CBBC51E2F Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAkzht0YACgkQitSsb3rl5xSWbwCgqWinTigQPRt1rs59Yc8Q6L3Q hQwAoL7U9dRbqYvpSM83xufNzTGxfMNA =U9hB -----END PGP SIGNATURE----- --------------enig8E0D5AF924FE652CBBC51E2F--