From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49316) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNnFD-0002HI-IT for qemu-devel@nongnu.org; Thu, 14 Jul 2016 16:28:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bNnFC-0002uU-91 for qemu-devel@nongnu.org; Thu, 14 Jul 2016 16:28:27 -0400 References: <1468524731-2306-1-git-send-email-vaibhav@digitalocean.com> From: Eric Blake Message-ID: <5787F5E5.1040209@redhat.com> Date: Thu, 14 Jul 2016 14:28:21 -0600 MIME-Version: 1.0 In-Reply-To: <1468524731-2306-1-git-send-email-vaibhav@digitalocean.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="CEbkwocDClVKwgs35PXVGCx5BiKHGDIaO" Subject: Re: [Qemu-devel] [PATCH v2] rbd: reload ceph config for block device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vaibhav Bhembre , qemu-devel@nongnu.org Cc: Josh Durgin , Jeff Cody , Kevin Wolf , Max Reitz , Luiz Capitulino , Markus Armbruster , qemu block This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --CEbkwocDClVKwgs35PXVGCx5BiKHGDIaO From: Eric Blake To: Vaibhav Bhembre , qemu-devel@nongnu.org Cc: Josh Durgin , Jeff Cody , Kevin Wolf , Max Reitz , Luiz Capitulino , Markus Armbruster , qemu block Message-ID: <5787F5E5.1040209@redhat.com> Subject: Re: [PATCH v2] rbd: reload ceph config for block device References: <1468524731-2306-1-git-send-email-vaibhav@digitalocean.com> In-Reply-To: <1468524731-2306-1-git-send-email-vaibhav@digitalocean.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/14/2016 01:32 PM, Vaibhav Bhembre wrote: > This patch adds ability to reload ceph configuration for an attached RB= D > block device. This is necessary for the cases where rebooting a VM and/= or > detaching-reattaching a RBD drive is not an easy option. Probably worth including qemu-block@nongnu.org if you resend this. I've added them in cc now, per the output of: scripts/get_maintainer.pl -f block/rbd >=20 > The reload mechanism relies on the bdrv_reopen_* calls to provide a tra= nsactional > guarantee (using 2PC) for pulling in new configuration parameters. In t= he _prepare > phase we do the grunt-work of creating and establishing new connection = and open > another instance of the same RBD image. If any issues are observed whil= e creating a > connection using the new parameters we _abort the reload. The original = connection to > the cluster is kept available and all ongoing I/O on it should be fine.= >=20 > Once the _prepare phase completes successfully we enter the _commit pha= se. In this phase > we simple move the I/O over to the new fd for the corresponding image w= e have already > created in the _prepare phase and reclaim the old rados I/O context and= connection. >=20 > It is important to note that because we want to use this feature when a= QEMU VM is already > running, we need to switch the logic to have values in ceph.conf overri= de the ones present > in the -drive file=3D* string in order for new changes to take place, f= or same keys present > in both places. >=20 > Signed-off-by: Vaibhav Bhembre >=20 > diff --git a/block/rbd.c b/block/rbd.c Your patch is missing the usual --- separator and diffstat that 'git format-patch' (and 'git send-email') normally produce. Without that, I don't have a good up-front idea on what it touches. Also, you titled this v2, in which case it's nice to mention what you changed since v1. You may want to look at http://wiki.qemu.org/Contribute/SubmitAPatch for other hints on writing a patch that is easier to review. > +++ b/qapi-schema.json > @@ -4317,3 +4317,16 @@ > # Since: 2.7 > ## > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU']= } > + > +## > +# @reload-rbd-config > +# > +# Reload the ceph config for a given RBD block device attached to the = VM. > +# > +# @node: Name of the node. > +# > +# Returns: nothing on success. > +# > +# Since: 2.7 v1 was posted June 17, before soft freeze on June 28, so this may still make hard freeze if someone picks it up before hard freeze on July 19. But we're getting close. > +++ b/qmp.c > @@ -707,3 +707,34 @@ ACPIOSTInfoList *qmp_query_acpi_ospm_status(Error = **errp) > =20 > return head; > } > + > +void qmp_reload_rbd_config(const char *node, Error **errp) > +{ > + BlockBackend *blk; > + BlockDriverState *bs; > + Error *local_err =3D NULL; > + int ret; > + > + blk =3D blk_by_name(node); > + if (!blk) { > + error_setg(errp, QERR_INVALID_PARAMETER, "node"); > + return; > + } > + We may want to rebase this on top of Kevin's series that adds qmp_get_root_bs() https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg03086.html > + bs =3D blk_bs(blk); > + if (!bs) { > + error_setg(errp, "no BDS found"); > + return; > + } > + > + ret =3D bdrv_reopen(bs, bdrv_get_flags(bs), &local_err); This seems pretty generic - would it be better to just have a general 'block-reopen' command, instead of making it specific to rbd? I'll let other block maintainers do a deeper review (I just focused on the interface). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --CEbkwocDClVKwgs35PXVGCx5BiKHGDIaO 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXh/XlAAoJEKeha0olJ0Nq3ZgH/0acm+5cfJvNpAvLE6w+C1TL O86w5Fe8NA3Up+ZwC5+hJV01fjU0KiOoHu/DR9oI1PlGAEfVnBfZPBqcCFDFI7sP D420WVV+8tyR07Qymg0jPb1KLlV/FMwjlso+qnGKp7x0HshrFSX4f8pq+ogJ7i7E dJGbaulZRfia5RfcTie+x+nDBE2nMWQilbtVTfsalhxz3ATB5mLn8BnclLTahQ1x /K+i81KJa5UOTJHdaXFG6u31E53TjNhjTO6due1DgAoSH9jI8RpKcjSIlZgv6NUD Jn7Ai1QVNu/dOI9e+lfpv04mA0w3G4qzH5l7nwqLwTuO5SgPQ/y4EMlkahubtCI= =xcW2 -----END PGP SIGNATURE----- --CEbkwocDClVKwgs35PXVGCx5BiKHGDIaO--